[Backport] Security issue 868592

M69 Mega-patch for 868592 fix

This CL is a collection of cherry-picks related to crbug.com/868592 fix.
Specifically, this is:
- The original mega-patch crrev.com/222c9ba7c6
- creis@ follow-up fix crrev.com/27986c7c955
- kouhei@ follow-up fix crrev.com/6be8b5a07bdf

The original change descriptions are captured below % Change-Id lines

---

Speculative crash fix for navigator.serviceworker access during unload

This should fix crash/caab6eb137e58385

This CL addresses the unhandled case in crrev.com/582126

TBR=falken@chromium.org

Bug: 881126, 868592
Reviewed-on: https://chromium-review.googlesource.com/1207781
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#589419}(cherry picked from commit 6be8b5a07bdfa95c37e2da9cace7d7d4b69b31b5)
Reviewed-on: https://chromium-review.googlesource.com/1212368
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/branch-heads/3545@{#2}
Cr-Branched-From: a2bbe9dedf867fccce6d8073dc8e9c864c662bfe-refs/heads/master@{#589377}

Speculative fix for additional History DocumentLoader crashes.

There is no guarantee that the DocumentLoader is always attached [1],
so let's introduce null checks in StateInternal and setScrollRestoration.

[1] The DocumentLoader may be detached while FrameLoader::PrepareForCommit.

BUG=879477, 872672

Reviewed-on: https://chromium-review.googlesource.com/1200075
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588227}
Speculative fix for History::ScrollRestorationInternal null deref

This is a speculative fix for crash reported on crbug.com/872672 .

There is no guarantee that the DocumentLoader is always attached [1],
so let's introduce a null check.

[1] The DocumentLoader may be detached while FrameLoader::PrepareForCommit.

Bug: 872672
Reviewed-on: https://chromium-review.googlesource.com/1171972
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582509}
(cherry picked from commit a2a107d712d0fb754cc03ccb36630fa3ddc90f14)

NavigatorServiceWorker: Avoid instantiating if being navigated away.

This CL fixes a clusterfuzz crash which fails to minimize.

Bug: 872320
Reviewed-on: https://chromium-review.googlesource.com/1170160
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582126}
(cherry picked from commit d995adf5879e71074e33983b0b038d21b3f3be43)

Flush microtask queue before commit

Bug: 868592
Reviewed-on: https://chromium-review.googlesource.com/1164148
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581124}
(cherry picked from commit 38894f66a9f1d3142746572da766de1b02f8e2ce)

Prevent promise reject to be sync scheduled during DocumentLoader detach
(% mod: revert fetch_manager.cc change)

(cherry picked from commit c415acc1e0cd8ec75e43bcb596f7bd76321f72f8)

Bug: 868592
Change-Id: I50029416f0441a9f09c538716684a01cb8af93e1
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1163235
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/master@{#580814}
Reviewed-on: https://chromium-review.googlesource.com/1184122
Cr-Original-Commit-Position: refs/branch-heads/3497@{#760}
Cr-Original-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
Reviewed-on: https://chromium-review.googlesource.com/1218183
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#938}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
Reviewed-by: Michal Klocek <michal.klocek@qt.io>

4 files changed, 73 insertions, 15 deletions

diff --git a/chromium/content/renderer/render_frame_impl.cc b/chromium/content/renderer/render_frame_impl.cc
index b7e06c2c288..34f7fb2b64b 100644
--- a/chromium/content/renderer/render_frame_impl.cc
+++ b/chromium/content/renderer/render_frame_impl.cc
@@ -3617,8 +3617,17 @@ blink::BlameContext* RenderFrameImpl::GetFrameBlameContext() {
 
 std::unique_ptr<blink::WebServiceWorkerProvider>
 RenderFrameImpl::CreateServiceWorkerProvider() {
+  // Bail-out if we are about to be navigated away.
+  // We check that DocumentLoader is attached since:
+  // - This serves as the signal since the DocumentLoader is detached in
+  //   FrameLoader::PrepareForCommit().
+  // - Creating ServiceWorkerProvider in
+  //   RenderFrameImpl::CreateServiceWorkerProvider() assumes that there is a
+  //   DocumentLoader attached to the frame.
+  if (!frame_->GetDocumentLoader())
+    return nullptr;
+
   // At this point we should have non-null data source.
-  DCHECK(frame_->GetDocumentLoader());
   if (!ChildThreadImpl::current())
     return nullptr;  // May be null in some tests.
   ServiceWorkerNetworkProvider* provider =
diff --git a/chromium/third_party/WebKit/Source/core/frame/History.cpp b/chromium/third_party/WebKit/Source/core/frame/History.cpp
index 8b1156ce437..cdb9bc62d4b 100644
--- a/chromium/third_party/WebKit/Source/core/frame/History.cpp
+++ b/chromium/third_party/WebKit/Source/core/frame/History.cpp
@@ -86,21 +86,30 @@ SerializedScriptValue* History::state(ExceptionState& exception_state) {
 }
 
 SerializedScriptValue* History::StateInternal() const {
-  if (!GetFrame())
+  LocalFrame* frame = GetFrame();
+  if (!frame)
     return nullptr;
 
-  if (HistoryItem* history_item =
-          GetFrame()->Loader().GetDocumentLoader()->GetHistoryItem()) {
-    return history_item->StateObject();
-  }
+  // TODO(kouhei, dcheng): The DocumentLoader null check should be unnecessary.
+  // Investigate and see if it can be removed.
+  DocumentLoader* document_loader = frame->Loader().GetDocumentLoader();
+  if (!document_loader)
+    return nullptr;
+
+  HistoryItem* history_item = document_loader->GetHistoryItem();
+  if (!history_item)
+    return nullptr;
 
-  return nullptr;
+  return history_item->StateObject();
 }
 
 void History::setScrollRestoration(const String& value,
                                    ExceptionState& exception_state) {
   DCHECK(value == "manual" || value == "auto");
-  if (!GetFrame() || !GetFrame()->Client()) {
+  // TODO(kouhei, dcheng): The DocumentLoader null check should be unnecessary.
+  // Investigate and see if it can be removed.
+  if (!GetFrame() || !GetFrame()->Client() ||
+      !GetFrame()->Loader().GetDocumentLoader()) {
     exception_state.ThrowSecurityError(
         "May not use a History object associated with a Document that is not "
         "fully active");
@@ -131,11 +140,23 @@ String History::scrollRestoration(ExceptionState& exception_state) {
 }
 
 HistoryScrollRestorationType History::ScrollRestorationInternal() const {
-  HistoryItem* history_item =
-      GetFrame() ? GetFrame()->Loader().GetDocumentLoader()->GetHistoryItem()
-                 : nullptr;
-  return history_item ? history_item->ScrollRestorationType()
-                      : kScrollRestorationAuto;
+  constexpr HistoryScrollRestorationType default_type = kScrollRestorationAuto;
+
+  LocalFrame* frame = GetFrame();
+  if (!frame)
+    return default_type;
+
+  // TODO(kouhei, dcheng): The DocumentLoader null check should be unnecessary.
+  // Investigate and see if it can be removed.
+  DocumentLoader* document_loader = frame->Loader().GetDocumentLoader();
+  if (!document_loader)
+    return default_type;
+
+  HistoryItem* history_item = document_loader->GetHistoryItem();
+  if (!history_item)
+    return default_type;
+
+  return history_item->ScrollRestorationType();
 }
 
 // TODO(crbug.com/394296): This is not the long-term fix to IPC flooding that we
diff --git a/chromium/third_party/WebKit/Source/core/loader/FrameLoader.cpp b/chromium/third_party/WebKit/Source/core/loader/FrameLoader.cpp
index f0b1f6e9d63..7d63d6594e1 100644
--- a/chromium/third_party/WebKit/Source/core/loader/FrameLoader.cpp
+++ b/chromium/third_party/WebKit/Source/core/loader/FrameLoader.cpp
@@ -80,7 +80,9 @@
 #include "platform/InstanceCounters.h"
 #include "platform/WebFrameScheduler.h"
 #include "platform/bindings/DOMWrapperWorld.h"
+#include "platform/bindings/Microtask.h"
 #include "platform/bindings/ScriptForbiddenScope.h"
+#include "platform/bindings/V8PerIsolateData.h"
 #include "platform/instrumentation/tracing/TraceEvent.h"
 #include "platform/loader/fetch/ResourceFetcher.h"
 #include "platform/loader/fetch/ResourceRequest.h"
@@ -1079,6 +1081,14 @@ bool FrameLoader::PrepareForCommit() {
   if (!frame_->Client())
     return false;
   DCHECK_EQ(provisional_document_loader_, pdl);
+
+  // Flush microtask queue so that they all run on pre-navigation context.
+  Microtask::PerformCheckpoint(V8PerIsolateData::MainThreadIsolate());
+
+  // Ensure that the frame_ hasn't detached from running the microtasks.
+  if (!frame_->Client())
+    return false;
+
   // No more events will be dispatched so detach the Document.
   // TODO(yoav): Should we also be nullifying domWindow's document (or
   // domWindow) since the doc is now detached?
diff --git a/chromium/third_party/WebKit/Source/modules/serviceworkers/NavigatorServiceWorker.cpp b/chromium/third_party/WebKit/Source/modules/serviceworkers/NavigatorServiceWorker.cpp
index b2eff50e339..637665bb459 100644
--- a/chromium/third_party/WebKit/Source/modules/serviceworkers/NavigatorServiceWorker.cpp
+++ b/chromium/third_party/WebKit/Source/modules/serviceworkers/NavigatorServiceWorker.cpp
@@ -18,9 +18,27 @@ namespace blink {
 NavigatorServiceWorker::NavigatorServiceWorker(Navigator& navigator) {}
 
 NavigatorServiceWorker* NavigatorServiceWorker::From(Document& document) {
-  if (!document.GetFrame() || !document.GetFrame()->DomWindow())
+  LocalFrame* frame = document.GetFrame();
+  if (!frame)
     return nullptr;
-  Navigator& navigator = *document.GetFrame()->DomWindow()->navigator();
+
+  // TODO(kouhei): Remove below after M72, since the check is now done in
+  // RenderFrameImpl::CreateServiceWorkerProvider instead.
+  //
+  // Bail-out if we are about to be navigated away.
+  // We check that DocumentLoader is attached since:
+  // - This serves as the signal since the DocumentLoader is detached in
+  //   FrameLoader::PrepareForCommit().
+  // - Creating ServiceWorkerProvider in
+  //   RenderFrameImpl::CreateServiceWorkerProvider() assumes that there is a
+  //   DocumentLoader attached to the frame.
+  if (!frame->Loader().GetDocumentLoader())
+    return nullptr;
+
+  LocalDOMWindow* dom_window = frame->DomWindow();
+  if (!dom_window)
+    return nullptr;
+  Navigator& navigator = *dom_window->navigator();
   return &From(navigator);
 }