[Sheriff] Revert "AppCache: Reduce dependencies on DocumentLoader from...

[Sheriff] Revert "AppCache: Reduce dependencies on DocumentLoader from ApplicationCacheHost for shared workers (4)"

This reverts commit 1ac215fe.

Reason for revert: Breaks ChromeDoNotTrackTest.FetchFromSharedWorker on Linux MSan, see
                   https://crbug.com/988997

Original change's description:
> AppCache: Reduce dependencies on DocumentLoader from ApplicationCacheHost for shared workers (4)
>
> In the current implementation, WorkerShadowPage provides DocumentLoader to
> ApplicationCacheHost for shared workers. This blocks WorkerShadowPage removal
> (see the issue). To unblock it, this series of CLs reduce dependencies on
> DocumentLoader from ApplicationCacheHost for shared workers.
>
> This CL...
>
> - moves WillStartLoading(), WillStartLoadingMainResource(),
>   IsApplicationCacheEnabled(), and |document_loader_| from ApplicationCacheHost
>   to ApplicationCacheHostForFrame.
> - introduces WebLocalFrameClient::GetAppCacheHostIDForSharedWorker() to take an
>   appcache host ID from WorkerShadowPage with bypassing DocumentLoader. This is
>   a stopgap implementation, and will be removed by follow-up CLs soon.
> - makes ApplicationCacheHostForSharedWorker binds itself with the backend in the
>   constructor instead of WillStartLoadingMainResource() that is a part of the
>   lifecycle of DocumentLoader.
>
> The follow-up CLs will create ApplicationCacheHostForSharedWorker in
> WebSharedWorkerImpl instead of DocumentLoader. That will completely remove
> DocumentLoader association from ApplicationCacheHost for shared workers.
>
> Bug: 982996
> Change-Id: I5beedd7600291a6f4867d35505e658b7f7194fbd
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1722932
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
> Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#682075}

TBR=mek@chromium.org,haraken@chromium.org,nhiroki@chromium.org

Change-Id: Id8437f48e455cefaafa8c031d682a7abc4f8997e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 982996, 988997
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1726033
Commit-Queue: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682278}

third_party/blink/public/web/web_local_frame_client.h

@@ -880,12 +880,6 @@ class BLINK_EXPORT WebLocalFrameClient {
   virtual WebLocalFrameClient::AppCacheType GetAppCacheType() {
     return WebLocalFrameClient::AppCacheType::kAppCacheForNone;
   }
-  // TODO(https://crbug.com/982996): This is a stopgap implementation for
-  // removing DocumentLoader dependencies from shared workers. Remove this once
-  // it's done.
-  virtual base::UnguessableToken GetAppCacheHostIDForSharedWorker() {
-    return base::UnguessableToken();
-  }
 };
 
 }  // namespace blink

third_party/blink/renderer/core/exported/local_frame_client_impl.cc

@@ -1270,12 +1270,6 @@ WebLocalFrameClient::AppCacheType LocalFrameClientImpl::GetAppCacheType() {
   return web_frame_->Client()->GetAppCacheType();
 }
 
-base::UnguessableToken
-LocalFrameClientImpl::GetAppCacheHostIDForSharedWorker() {
-  DCHECK(web_frame_->Client());
-  return web_frame_->Client()->GetAppCacheHostIDForSharedWorker();
-}
-
 STATIC_ASSERT_ENUM(DownloadCrossOriginRedirects::kFollow,
                    WebLocalFrameClient::CrossOriginRedirects::kFollow);
 STATIC_ASSERT_ENUM(DownloadCrossOriginRedirects::kNavigate,

third_party/blink/renderer/core/exported/local_frame_client_impl.h

@@ -329,7 +329,6 @@ class LocalFrameClientImpl final : public LocalFrameClient {
   void UpdateSubresourceFactory(
       std::unique_ptr<blink::URLLoaderFactoryBundleInfo> info) override;
   WebLocalFrameClient::AppCacheType GetAppCacheType() override;
-  base::UnguessableToken GetAppCacheHostIDForSharedWorker() override;
 
  private:
   struct DocumentInterfaceBrokerForwarderTraits {

third_party/blink/renderer/core/exported/web_shared_worker_impl.cc

@@ -225,8 +225,6 @@ void WebSharedWorkerImpl::StartWorkerContext(
   shadow_page_ = std::make_unique<WorkerShadowPage>(
       this, std::move(loader_factory), std::move(privacy_preferences),
       appcache_host_id_);
-  // TODO(https://crbug.com/982996): Create ApplicationCacheHostForSharedWorker
-  // here without depending on WorkerShadowPage and DocumentLoader.
 
   // If we were asked to pause worker context on start and wait for debugger
   // then now is a good time to do that.

third_party/blink/renderer/core/exported/worker_shadow_page.cc

@@ -70,6 +70,7 @@ void WorkerShadowPage::Initialize(const KURL& script_url) {
   std::unique_ptr<WebNavigationParams> params =
       WebNavigationParams::CreateWithHTMLBuffer(
           SharedBuffer::Create(content.c_str(), content.length()), script_url);
+  params->appcache_host_id = appcache_host_id_;
   main_frame_->GetFrame()->Loader().CommitNavigation(std::move(params),
                                                      nullptr /* extra_data */);
 }

third_party/blink/renderer/core/exported/worker_shadow_page.h

@@ -75,9 +75,6 @@ class CORE_EXPORT WorkerShadowPage : public WebLocalFrameClient {
   WebLocalFrameClient::AppCacheType GetAppCacheType() override {
     return client_->GetAppCacheType();
   }
-  base::UnguessableToken GetAppCacheHostIDForSharedWorker() override {
-    return appcache_host_id_;
-  }
 
   Document* GetDocument() { return main_frame_->GetFrame()->GetDocument(); }
   WebSettings* GetSettings() { return web_view_->GetSettings(); }

third_party/blink/renderer/core/frame/local_frame_client.h

@@ -534,12 +534,6 @@ class CORE_EXPORT LocalFrameClient : public FrameClient {
   virtual WebLocalFrameClient::AppCacheType GetAppCacheType() {
     return WebLocalFrameClient::AppCacheType::kAppCacheForNone;
   }
-  // TODO(https://crbug.com/982996): This is a stopgap implementation for
-  // removing DocumentLoader dependencies from shared workers. Remove this once
-  // it's done.
-  virtual base::UnguessableToken GetAppCacheHostIDForSharedWorker() {
-    return base::UnguessableToken();
-  }
 };
 
 }  // namespace blink

third_party/blink/renderer/core/loader/appcache/application_cache_host.cc

@@ -41,6 +41,7 @@
 #include "third_party/blink/renderer/core/frame/hosts_using_features.h"
 #include "third_party/blink/renderer/core/frame/local_frame.h"
 #include "third_party/blink/renderer/core/frame/local_frame_client.h"
+#include "third_party/blink/renderer/core/frame/settings.h"
 #include "third_party/blink/renderer/core/inspector/inspector_application_cache_agent.h"
 #include "third_party/blink/renderer/core/loader/appcache/application_cache.h"
 #include "third_party/blink/renderer/core/loader/appcache/application_cache_host_for_frame.h"
@@ -69,9 +70,6 @@ mojom::blink::DocumentInterfaceBroker* GetDocumentInterfaceBroker(
 
 }  // namespace
 
-// TODO(https://crbug.com/982996): Remove this creation function, and instead
-// directly create an appcache host for frame in DocumentLoader, and for shared
-// workers in WebSharedWorkerImpl.
 ApplicationCacheHost* ApplicationCacheHost::Create(
     DocumentLoader* document_loader) {
   DCHECK(document_loader);
@@ -88,26 +86,53 @@ ApplicationCacheHost* ApplicationCacheHost::Create(
           local_frame->GetTaskRunner(TaskType::kNetworking));
     case WebLocalFrameClient::AppCacheType::kAppCacheForSharedWorker:
       return MakeGarbageCollected<ApplicationCacheHostForSharedWorker>(
-          local_frame->Client()->GetAppCacheHostIDForSharedWorker(),
-          Thread::Current()->GetTaskRunner());
+          document_loader, Thread::Current()->GetTaskRunner());
     default:
-      return MakeGarbageCollected<ApplicationCacheHost>(
-          /*interface_broker=*/nullptr, /*task_runner=*/nullptr);
+      return MakeGarbageCollected<ApplicationCacheHost>(document_loader,
+                                                        nullptr, nullptr);
   }
   return nullptr;
 }
 
+// We provide a custom implementation of this class that calls out to the
+// embedding application instead of using WebCore's built in appcache system.
+// This file replaces webcore/appcache/ApplicationCacheHost.cpp in our build.
+
 ApplicationCacheHost::ApplicationCacheHost(
+    DocumentLoader* document_loader,
     mojom::blink::DocumentInterfaceBroker* interface_broker,
     scoped_refptr<base::SingleThreadTaskRunner> task_runner)
-    : task_runner_(std::move(task_runner)),
+    : document_loader_(document_loader),
+      task_runner_(std::move(task_runner)),
       interface_broker_(interface_broker) {}
 
 ApplicationCacheHost::~ApplicationCacheHost() = default;
 
-void ApplicationCacheHost::Detach() {
+void ApplicationCacheHost::WillStartLoading(ResourceRequest& request) {
+  if (!IsApplicationCacheEnabled() || !backend_host_.is_bound())
+    return;
+  const base::UnguessableToken& host_id = GetHostID();
+  if (!host_id.is_empty())
+    request.SetAppCacheHostID(host_id);
+}
+
+void ApplicationCacheHost::WillStartLoadingMainResource(DocumentLoader* loader,
+                                                        const KURL& url,
+                                                        const String& method) {
+  if (!IsApplicationCacheEnabled())
+    return;
+
+  // We defer binding to backend to avoid unnecessary binding around creating
+  // empty documents. At this point, we're initiating a main resource load for
+  // the document, so its for real.
+  BindBackend();
+}
+
+void ApplicationCacheHost::DetachFromDocumentLoader() {
+  // Detach from the owning DocumentLoader and close mojo pipes.
   receiver_.reset();
   backend_host_.reset();
+  document_loader_ = nullptr;
 }
 
 ApplicationCacheHost::CacheInfo ApplicationCacheHost::ApplicationCacheInfo() {
@@ -125,11 +150,6 @@ const base::UnguessableToken& ApplicationCacheHost::GetHostID() const {
   return host_id_;
 }
 
-void ApplicationCacheHost::SetHostID(const base::UnguessableToken& host_id) {
-  DCHECK(!host_id.is_empty());
-  host_id_ = host_id;
-}
-
 void ApplicationCacheHost::SelectCacheForSharedWorker(
     int64_t app_cache_id,
     base::OnceClosure completion_callback) {
@@ -165,6 +185,14 @@ void ApplicationCacheHost::Abort() {
   // This is not implemented intentionally. See https://crbug.com/175063
 }
 
+bool ApplicationCacheHost::IsApplicationCacheEnabled() {
+  DCHECK(document_loader_->GetFrame());
+  return document_loader_->GetFrame()->GetSettings() &&
+         document_loader_->GetFrame()
+             ->GetSettings()
+             ->GetOfflineWebApplicationCacheEnabled();
+}
+
 void ApplicationCacheHost::CacheSelected(mojom::blink::AppCacheInfoPtr info) {
   if (!backend_host_.is_bound())
     return;
@@ -287,7 +315,11 @@ bool ApplicationCacheHost::BindBackend() {
   if (!task_runner_)
     return false;
 
-  DCHECK(!host_id_.is_empty());
+  // PlzNavigate: The browser passes the ID to be used.
+  if (!document_loader_->AppcacheHostId().is_empty())
+    host_id_ = document_loader_->AppcacheHostId();
+  else
+    host_id_ = base::UnguessableToken::Create();
 
   mojo::PendingRemote<mojom::blink::AppCacheFrontend> frontend_remote;
   receiver_.Bind(frontend_remote.InitWithNewPipeAndPassReceiver(),
@@ -317,4 +349,8 @@ bool ApplicationCacheHost::BindBackend() {
   return true;
 }
 
+void ApplicationCacheHost::Trace(blink::Visitor* visitor) {
+  visitor->Trace(document_loader_);
+}
+
 }  // namespace blink

third_party/blink/renderer/core/loader/appcache/application_cache_host.h

@@ -64,10 +64,11 @@ class CORE_EXPORT ApplicationCacheHost
   // |interface_broker| can be null for workers and |task_runner| is null for
   // kAppCacheForNone.
   explicit ApplicationCacheHost(
+      DocumentLoader* document_loader,
       mojom::blink::DocumentInterfaceBroker* interface_broker,
       scoped_refptr<base::SingleThreadTaskRunner> task_runner);
   ~ApplicationCacheHost() override;
-  virtual void Detach();
+  virtual void DetachFromDocumentLoader();
 
   struct CacheInfo {
     STACK_ALLOCATED();
@@ -91,6 +92,9 @@ class CORE_EXPORT ApplicationCacheHost
     int64_t padding_sizes_ = 0;
   };
 
+  // Annotate request for ApplicationCache.
+  void WillStartLoading(ResourceRequest&);
+
   mojom::blink::AppCacheStatus GetStatus() const;
   virtual bool Update() { return false; }
   virtual bool SwapCache() { return false; }
@@ -102,10 +106,7 @@ class CORE_EXPORT ApplicationCacheHost
 
   void FillResourceList(Vector<mojom::blink::AppCacheResourceInfo>*);
   CacheInfo ApplicationCacheInfo();
-
   const base::UnguessableToken& GetHostID() const;
-  void SetHostID(const base::UnguessableToken& host_id);
-
   void SelectCacheForSharedWorker(int64_t app_cache_id,
                                   base::OnceClosure completion_callback);
 
@@ -121,23 +122,21 @@ class CORE_EXPORT ApplicationCacheHost
   void SetSubresourceFactory(
       network::mojom::blink::URLLoaderFactoryPtr url_loader_factory) override {}
 
-  virtual void WillStartLoading(ResourceRequest&) {}
   virtual void WillStartLoadingMainResource(DocumentLoader* loader,
                                             const KURL& url,
-                                            const String& method) {}
+                                            const String& method);
   virtual void SelectCacheWithoutManifest() {}
   virtual void SelectCacheWithManifest(const KURL& manifest_url) {}
   virtual void DidReceiveResponseForMainResource(const ResourceResponse&) {}
-  virtual void Trace(blink::Visitor*) {}
+  virtual void Trace(blink::Visitor*);
 
  protected:
+  DocumentLoader* GetDocumentLoader() const { return document_loader_; }
+
   mojo::Remote<mojom::blink::AppCacheHost> backend_host_;
   mojom::blink::AppCacheStatus status_ =
       mojom::blink::AppCacheStatus::APPCACHE_STATUS_UNCACHED;
 
-  // Non-empty |host_id_| must be set before calling this function.
-  bool BindBackend();
-
  private:
   virtual void NotifyApplicationCache(mojom::AppCacheEventID,
                                       int progress_total,
@@ -148,6 +147,11 @@ class CORE_EXPORT ApplicationCacheHost
                                       const String& error_message) {}
 
   void GetAssociatedCacheInfo(CacheInfo* info);
+  bool IsApplicationCacheEnabled();
+  bool BindBackend();
+
+  // TODO(https://crbug.com/982996): Move this to ApplicationCacheHostForFrame.
+  Member<DocumentLoader> document_loader_;
 
   mojo::Receiver<mojom::blink::AppCacheFrontend> receiver_{this};
   base::UnguessableToken host_id_;

third_party/blink/renderer/core/loader/appcache/application_cache_host_for_frame.cc

@@ -10,10 +10,8 @@
 #include "third_party/blink/renderer/core/events/application_cache_error_event.h"
 #include "third_party/blink/renderer/core/events/progress_event.h"
 #include "third_party/blink/renderer/core/frame/local_frame_client.h"
-#include "third_party/blink/renderer/core/frame/settings.h"
 #include "third_party/blink/renderer/core/inspector/console_message.h"
 #include "third_party/blink/renderer/core/loader/appcache/application_cache.h"
-#include "third_party/blink/renderer/core/loader/document_loader.h"
 #include "third_party/blink/renderer/core/page/page.h"
 #include "third_party/blink/renderer/core/probe/core_probes.h"
 #include "third_party/blink/renderer/platform/web_test_support.h"
@@ -45,13 +43,13 @@ ApplicationCacheHostForFrame::ApplicationCacheHostForFrame(
     DocumentLoader* document_loader,
     mojom::blink::DocumentInterfaceBroker* interface_broker,
     scoped_refptr<base::SingleThreadTaskRunner> task_runner)
-    : ApplicationCacheHost(interface_broker, std::move(task_runner)),
-      local_frame_(document_loader->GetFrame()),
-      document_loader_(document_loader) {}
+    : ApplicationCacheHost(document_loader,
+                           interface_broker,
+                           std::move(task_runner)),
+      local_frame_(document_loader->GetFrame()) {}
 
-void ApplicationCacheHostForFrame::Detach() {
-  ApplicationCacheHost::Detach();
-  document_loader_ = nullptr;
+void ApplicationCacheHostForFrame::DetachFromDocumentLoader() {
+  ApplicationCacheHost::DetachFromDocumentLoader();
   SetApplicationCache(nullptr);
 }
 
@@ -82,7 +80,7 @@ bool ApplicationCacheHostForFrame::SwapCache() {
   if (!success)
     return false;
   backend_host_->GetStatus(&status_);
-  probe::UpdateApplicationCacheStatus(document_loader_->GetFrame());
+  probe::UpdateApplicationCacheStatus(GetDocumentLoader()->GetFrame());
   return true;
 }
 
@@ -134,33 +132,14 @@ void ApplicationCacheHostForFrame::SetSubresourceFactory(
       std::move(pending_factories));
 }
 
-void ApplicationCacheHostForFrame::WillStartLoading(ResourceRequest& request) {
-  if (!IsApplicationCacheEnabled() || !backend_host_.is_bound())
-    return;
-  const base::UnguessableToken& host_id = GetHostID();
-  if (!host_id.is_empty())
-    request.SetAppCacheHostID(host_id);
-}
-
 void ApplicationCacheHostForFrame::WillStartLoadingMainResource(
     DocumentLoader* loader,
     const KURL& url,
     const String& method) {
-  if (!IsApplicationCacheEnabled())
+  ApplicationCacheHost::WillStartLoadingMainResource(loader, url, method);
+  if (!backend_host_.is_bound())
     return;
 
-  // PlzNavigate: The browser passes the ID to be used.
-  DCHECK(GetHostID().is_empty());
-  if (!document_loader_->AppcacheHostId().is_empty())
-    SetHostID(document_loader_->AppcacheHostId());
-  else
-    SetHostID(base::UnguessableToken::Create());
-
-  // We defer binding to backend to avoid unnecessary binding around creating
-  // empty documents. At this point, we're initiating a main resource load for
-  // the document, so its for real.
-  BindBackend();
-
   original_main_resource_url_ = ClearUrlRef(url);
   is_get_method_ = (method == kHttpGETMethod);
   DCHECK(method == method.UpperASCII());
@@ -205,7 +184,7 @@ void ApplicationCacheHostForFrame::SelectCacheWithoutManifest() {
 
 void ApplicationCacheHostForFrame::SelectCacheWithManifest(
     const KURL& manifest_url) {
-  LocalFrame* frame = document_loader_->GetFrame();
+  LocalFrame* frame = GetDocumentLoader()->GetFrame();
   Document* document = frame->GetDocument();
   if (document->IsSandboxed(WebSandboxFlags::kOrigin)) {
     // Prevent sandboxes from establishing application caches.
@@ -296,7 +275,6 @@ void ApplicationCacheHostForFrame::DidReceiveResponseForMainResource(
 void ApplicationCacheHostForFrame::Trace(blink::Visitor* visitor) {
   visitor->Trace(dom_application_cache_);
   visitor->Trace(local_frame_);
-  visitor->Trace(document_loader_);
   ApplicationCacheHost::Trace(visitor);
 }
 
@@ -309,7 +287,7 @@ void ApplicationCacheHostForFrame::NotifyApplicationCache(
     int error_status,
     const String& error_message) {
   if (id != mojom::AppCacheEventID::APPCACHE_PROGRESS_EVENT) {
-    probe::UpdateApplicationCacheStatus(document_loader_->GetFrame());
+    probe::UpdateApplicationCacheStatus(GetDocumentLoader()->GetFrame());
   }
 
   if (defers_events_) {
@@ -351,12 +329,4 @@ void ApplicationCacheHostForFrame::DispatchDOMEvent(
   dom_application_cache_->DispatchEvent(*event);
 }
 
-bool ApplicationCacheHostForFrame::IsApplicationCacheEnabled() {
-  DCHECK(document_loader_->GetFrame());
-  return document_loader_->GetFrame()->GetSettings() &&
-         document_loader_->GetFrame()
-             ->GetSettings()
-             ->GetOfflineWebApplicationCacheEnabled();
-}
-
 }  // namespace blink

third_party/blink/renderer/core/loader/appcache/application_cache_host_for_frame.h

@@ -20,7 +20,7 @@ class CORE_EXPORT ApplicationCacheHostForFrame : public ApplicationCacheHost {
       scoped_refptr<base::SingleThreadTaskRunner> task_runner);
 
   // ApplicationCacheHost:
-  void Detach() override;
+  void DetachFromDocumentLoader() override;
   bool Update() override;
   bool SwapCache() override;
   void SetApplicationCache(ApplicationCache*) override;
@@ -32,7 +32,6 @@ class CORE_EXPORT ApplicationCacheHostForFrame : public ApplicationCacheHost {
   void SetSubresourceFactory(
       network::mojom::blink::URLLoaderFactoryPtr url_loader_factory) override;
 
-  void WillStartLoading(ResourceRequest&) override;
   void WillStartLoadingMainResource(DocumentLoader* loader,
                                     const KURL& url,
                                     const String& method) override;
@@ -85,13 +84,9 @@ class CORE_EXPORT ApplicationCacheHostForFrame : public ApplicationCacheHost {
                         int error_status,
                         const String& error_message);
 
-  bool IsApplicationCacheEnabled();
-
   WeakMember<ApplicationCache> dom_application_cache_ = nullptr;
 
   Member<LocalFrame> local_frame_;
-  Member<DocumentLoader> document_loader_;
-
   bool is_get_method_ = false;
   bool was_select_cache_called_ = false;
   IsNewMasterEntry is_new_master_entry_ = MAYBE_NEW_ENTRY;

third_party/blink/renderer/core/loader/appcache/application_cache_host_for_shared_worker.cc

@@ -7,14 +7,11 @@
 namespace blink {
 
 ApplicationCacheHostForSharedWorker::ApplicationCacheHostForSharedWorker(
-    const base::UnguessableToken& appcache_host_id,
+    DocumentLoader* document_loader,
     scoped_refptr<base::SingleThreadTaskRunner> task_runner)
-    : ApplicationCacheHost(nullptr, /* interface_broker */
-                           std::move(task_runner)) {
-  SetHostID(appcache_host_id ? appcache_host_id
-                             : base::UnguessableToken::Create());
-  BindBackend();
-}
+    : ApplicationCacheHost(document_loader,
+                           nullptr, /* interface_broker */
+                           std::move(task_runner)) {}
 
 ApplicationCacheHostForSharedWorker::~ApplicationCacheHostForSharedWorker() =
     default;

third_party/blink/renderer/core/loader/appcache/application_cache_host_for_shared_worker.h

@@ -12,7 +12,7 @@ namespace blink {
 class ApplicationCacheHostForSharedWorker final : public ApplicationCacheHost {
  public:
   ApplicationCacheHostForSharedWorker(
-      const base::UnguessableToken& appcache_host_id,
+      DocumentLoader* document_loader,
       scoped_refptr<base::SingleThreadTaskRunner> task_runner);
   ~ApplicationCacheHostForSharedWorker() override;
 

third_party/blink/renderer/core/loader/document_loader.cc

@@ -1083,7 +1083,7 @@ void DocumentLoader::DetachFromFrame(bool flush_microtask_queue) {
     return;
 
   if (application_cache_host_) {
-    application_cache_host_->Detach();
+    application_cache_host_->DetachFromDocumentLoader();
     application_cache_host_.Clear();
   }
   service_worker_network_provider_ = nullptr;