AppCache: Reduce dependencies on DocumentLoader from ApplicationCacheHost for shared workers (3)

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 ApplicationCacheHost::NotifyApplicationCache() to
ApplicationCacheHostForFrame. That function is called for dispatching DOMEvent
related appcache, but it's not necessary for shared workers because shared
workers don't have any appcache interfaces.

Bug: 982996
Change-Id: I019ee6df5850408b160bd09663be246071bb0c48
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1722135Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#681900}

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

@@ -37,8 +37,6 @@
 #include "third_party/blink/public/mojom/appcache/appcache_info.mojom-blink.h"
 #include "third_party/blink/public/platform/interface_provider.h"
 #include "third_party/blink/public/platform/platform.h"
-#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/deprecation.h"
 #include "third_party/blink/renderer/core/frame/hosts_using_features.h"
 #include "third_party/blink/renderer/core/frame/local_frame.h"
@@ -52,7 +50,6 @@
 #include "third_party/blink/renderer/core/loader/frame_load_request.h"
 #include "third_party/blink/renderer/core/loader/frame_loader.h"
 #include "third_party/blink/renderer/core/page/frame_tree.h"
-#include "third_party/blink/renderer/core/probe/core_probes.h"
 #include "third_party/blink/renderer/platform/bindings/exception_state.h"
 #include "third_party/blink/renderer/platform/wtf/assertions.h"
 
@@ -131,43 +128,13 @@ void ApplicationCacheHost::WillStartLoadingMainResource(DocumentLoader* loader,
   BindBackend();
 }
 
-void ApplicationCacheHost::SetApplicationCache(
-    ApplicationCache* dom_application_cache) {
-  DCHECK(!dom_application_cache_ || !dom_application_cache);
-  dom_application_cache_ = dom_application_cache;
-}
-
 void ApplicationCacheHost::DetachFromDocumentLoader() {
   // Detach from the owning DocumentLoader and close mojo pipes.
-  SetApplicationCache(nullptr);
   receiver_.reset();
   backend_host_.reset();
   document_loader_ = nullptr;
 }
 
-void ApplicationCacheHost::NotifyApplicationCache(
-    mojom::AppCacheEventID id,
-    int progress_total,
-    int progress_done,
-    mojom::AppCacheErrorReason error_reason,
-    const String& error_url,
-    int error_status,
-    const String& error_message) {
-  if (id != mojom::AppCacheEventID::APPCACHE_PROGRESS_EVENT) {
-    probe::UpdateApplicationCacheStatus(document_loader_->GetFrame());
-  }
-
-  if (defers_events_) {
-    // Event dispatching is deferred until document.onload has fired.
-    deferred_events_.push_back(DeferredEvent(id, progress_total, progress_done,
-                                             error_reason, error_url,
-                                             error_status, error_message));
-    return;
-  }
-  DispatchDOMEvent(id, progress_total, progress_done, error_reason, error_url,
-                   error_status, error_message);
-}
-
 ApplicationCacheHost::CacheInfo ApplicationCacheHost::ApplicationCacheInfo() {
   if (!backend_host_.is_bound())
     return CacheInfo();
@@ -208,46 +175,6 @@ void ApplicationCacheHost::FillResourceList(
     resources->emplace_back(std::move(*b));
 }
 
-void ApplicationCacheHost::StopDeferringEvents() {
-  for (unsigned i = 0; i < deferred_events_.size(); ++i) {
-    const DeferredEvent& deferred = deferred_events_[i];
-    DispatchDOMEvent(deferred.event_id, deferred.progress_total,
-                     deferred.progress_done, deferred.error_reason,
-                     deferred.error_url, deferred.error_status,
-                     deferred.error_message);
-  }
-  deferred_events_.clear();
-  defers_events_ = false;
-}
-
-void ApplicationCacheHost::DispatchDOMEvent(
-    mojom::AppCacheEventID id,
-    int progress_total,
-    int progress_done,
-    mojom::AppCacheErrorReason error_reason,
-    const String& error_url,
-    int error_status,
-    const String& error_message) {
-  // Don't dispatch an event if the window is detached.
-  if (!dom_application_cache_ || !dom_application_cache_->DomWindow())
-    return;
-
-  const AtomicString& event_type = ApplicationCache::ToEventType(id);
-  if (event_type.IsEmpty())
-    return;
-  Event* event = nullptr;
-  if (id == mojom::AppCacheEventID::APPCACHE_PROGRESS_EVENT) {
-    event =
-        ProgressEvent::Create(event_type, true, progress_done, progress_total);
-  } else if (id == mojom::AppCacheEventID::APPCACHE_ERROR_EVENT) {
-    event = MakeGarbageCollected<ApplicationCacheErrorEvent>(
-        error_reason, error_url, error_status, error_message);
-  } else {
-    event = Event::Create(event_type);
-  }
-  dom_application_cache_->DispatchEvent(*event);
-}
-
 mojom::AppCacheStatus ApplicationCacheHost::GetStatus() const {
   if (!backend_host_.is_bound())
     return mojom::AppCacheStatus::APPCACHE_STATUS_UNCACHED;
@@ -423,7 +350,6 @@ bool ApplicationCacheHost::BindBackend() {
 }
 
 void ApplicationCacheHost::Trace(blink::Visitor* visitor) {
-  visitor->Trace(dom_application_cache_);
   visitor->Trace(document_loader_);
 }
 

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

@@ -53,6 +53,9 @@ class ApplicationCache;
 class DocumentLoader;
 class ResourceRequest;
 
+// TODO(nhiroki): Move virtual functions in this class into
+// ApplicationCacheHostForFrame after making DocumentLoader own only
+// ApplicationCacheHostForFrame (not own ApplicationCacheHostForSharedWorker).
 class CORE_EXPORT ApplicationCacheHost
     : public GarbageCollectedFinalized<ApplicationCacheHost>,
       public mojom::blink::AppCacheFrontend {
@@ -65,7 +68,7 @@ class CORE_EXPORT ApplicationCacheHost
       mojom::blink::DocumentInterfaceBroker* interface_broker,
       scoped_refptr<base::SingleThreadTaskRunner> task_runner);
   ~ApplicationCacheHost() override;
-  void DetachFromDocumentLoader();
+  virtual void DetachFromDocumentLoader();
 
   struct CacheInfo {
     STACK_ALLOCATED();
@@ -97,10 +100,9 @@ class CORE_EXPORT ApplicationCacheHost
   virtual bool SwapCache() { return false; }
   void Abort();
 
-  void SetApplicationCache(ApplicationCache*);
+  virtual void SetApplicationCache(ApplicationCache*) {}
 
-  void
-  StopDeferringEvents();  // Also raises the events that have been queued up.
+  virtual void StopDeferringEvents() {}
 
   void FillResourceList(Vector<mojom::blink::AppCacheResourceInfo>*);
   CacheInfo ApplicationCacheInfo();
@@ -120,9 +122,6 @@ class CORE_EXPORT ApplicationCacheHost
   void SetSubresourceFactory(
       network::mojom::blink::URLLoaderFactoryPtr url_loader_factory) override {}
 
-  // TODO(nhiroki): Move these virtual functions into
-  // ApplicationCacheHostForFrame after making DocumentLoader own only
-  // ApplicationCacheHostForFrame (not own ApplicationCacheHostForSharedWorker).
   virtual void WillStartLoadingMainResource(DocumentLoader* loader,
                                             const KURL& url,
                                             const String& method);
@@ -139,57 +138,21 @@ class CORE_EXPORT ApplicationCacheHost
       mojom::blink::AppCacheStatus::APPCACHE_STATUS_UNCACHED;
 
  private:
-  void NotifyApplicationCache(mojom::AppCacheEventID,
-                              int progress_total,
-                              int progress_done,
-                              mojom::AppCacheErrorReason,
-                              const String& error_url,
-                              int error_status,
-                              const String& error_message);
+  virtual void NotifyApplicationCache(mojom::AppCacheEventID,
+                                      int progress_total,
+                                      int progress_done,
+                                      mojom::AppCacheErrorReason,
+                                      const String& error_url,
+                                      int error_status,
+                                      const String& error_message) {}
 
   void GetAssociatedCacheInfo(CacheInfo* info);
   bool IsApplicationCacheEnabled();
   bool BindBackend();
-  void DispatchDOMEvent(mojom::AppCacheEventID,
-                        int progress_total,
-                        int progress_done,
-                        mojom::AppCacheErrorReason,
-                        const String& error_url,
-                        int error_status,
-                        const String& error_message);
-
-  struct DeferredEvent {
-    mojom::AppCacheEventID event_id;
-    int progress_total;
-    int progress_done;
-    mojom::AppCacheErrorReason error_reason;
-    String error_url;
-    int error_status;
-    String error_message;
-    DeferredEvent(mojom::AppCacheEventID id,
-                  int progress_total,
-                  int progress_done,
-                  mojom::AppCacheErrorReason error_reason,
-                  const String& error_url,
-                  int error_status,
-                  const String& error_message)
-        : event_id(id),
-          progress_total(progress_total),
-          progress_done(progress_done),
-          error_reason(error_reason),
-          error_url(error_url),
-          error_status(error_status),
-          error_message(error_message) {}
-  };
-
-  WeakMember<ApplicationCache> dom_application_cache_ = nullptr;
 
   // TODO(https://crbug.com/982996): Move this to ApplicationCacheHostForFrame.
   Member<DocumentLoader> document_loader_;
 
-  bool defers_events_ =
-      true;  // Events are deferred until after document onload.
-  Vector<DeferredEvent> deferred_events_;
   mojo::Receiver<mojom::blink::AppCacheFrontend> receiver_{this};
   base::UnguessableToken host_id_;
   mojom::blink::AppCacheInfo cache_info_;

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

@@ -7,8 +7,11 @@
 #include "third_party/blink/public/common/loader/url_loader_factory_bundle.h"
 #include "third_party/blink/public/platform/platform.h"
 #include "third_party/blink/renderer/core/dom/document.h"
+#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/inspector/console_message.h"
+#include "third_party/blink/renderer/core/loader/appcache/application_cache.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,6 +48,11 @@ ApplicationCacheHostForFrame::ApplicationCacheHostForFrame(
                            std::move(task_runner)),
       local_frame_(document_loader->GetFrame()) {}
 
+void ApplicationCacheHostForFrame::DetachFromDocumentLoader() {
+  ApplicationCacheHost::DetachFromDocumentLoader();
+  SetApplicationCache(nullptr);
+}
+
 bool ApplicationCacheHostForFrame::Update() {
   if (!backend_host_.is_bound())
     return false;
@@ -76,6 +84,24 @@ bool ApplicationCacheHostForFrame::SwapCache() {
   return true;
 }
 
+void ApplicationCacheHostForFrame::SetApplicationCache(
+    ApplicationCache* dom_application_cache) {
+  DCHECK(!dom_application_cache_ || !dom_application_cache);
+  dom_application_cache_ = dom_application_cache;
+}
+
+void ApplicationCacheHostForFrame::StopDeferringEvents() {
+  for (unsigned i = 0; i < deferred_events_.size(); ++i) {
+    const DeferredEvent& deferred = deferred_events_[i];
+    DispatchDOMEvent(deferred.event_id, deferred.progress_total,
+                     deferred.progress_done, deferred.error_reason,
+                     deferred.error_url, deferred.error_status,
+                     deferred.error_message);
+  }
+  deferred_events_.clear();
+  defers_events_ = false;
+}
+
 void ApplicationCacheHostForFrame::LogMessage(
     mojom::blink::ConsoleMessageLevel log_level,
     const String& message) {
@@ -247,8 +273,60 @@ void ApplicationCacheHostForFrame::DidReceiveResponseForMainResource(
 }
 
 void ApplicationCacheHostForFrame::Trace(blink::Visitor* visitor) {
+  visitor->Trace(dom_application_cache_);
   visitor->Trace(local_frame_);
   ApplicationCacheHost::Trace(visitor);
 }
 
+void ApplicationCacheHostForFrame::NotifyApplicationCache(
+    mojom::AppCacheEventID id,
+    int progress_total,
+    int progress_done,
+    mojom::AppCacheErrorReason error_reason,
+    const String& error_url,
+    int error_status,
+    const String& error_message) {
+  if (id != mojom::AppCacheEventID::APPCACHE_PROGRESS_EVENT) {
+    probe::UpdateApplicationCacheStatus(GetDocumentLoader()->GetFrame());
+  }
+
+  if (defers_events_) {
+    // Event dispatching is deferred until document.onload has fired.
+    deferred_events_.push_back(DeferredEvent(id, progress_total, progress_done,
+                                             error_reason, error_url,
+                                             error_status, error_message));
+    return;
+  }
+  DispatchDOMEvent(id, progress_total, progress_done, error_reason, error_url,
+                   error_status, error_message);
+}
+
+void ApplicationCacheHostForFrame::DispatchDOMEvent(
+    mojom::AppCacheEventID id,
+    int progress_total,
+    int progress_done,
+    mojom::AppCacheErrorReason error_reason,
+    const String& error_url,
+    int error_status,
+    const String& error_message) {
+  // Don't dispatch an event if the window is detached.
+  if (!dom_application_cache_ || !dom_application_cache_->DomWindow())
+    return;
+
+  const AtomicString& event_type = ApplicationCache::ToEventType(id);
+  if (event_type.IsEmpty())
+    return;
+  Event* event = nullptr;
+  if (id == mojom::AppCacheEventID::APPCACHE_PROGRESS_EVENT) {
+    event =
+        ProgressEvent::Create(event_type, true, progress_done, progress_total);
+  } else if (id == mojom::AppCacheEventID::APPCACHE_ERROR_EVENT) {
+    event = MakeGarbageCollected<ApplicationCacheErrorEvent>(
+        error_reason, error_url, error_status, error_message);
+  } else {
+    event = Event::Create(event_type);
+  }
+  dom_application_cache_->DispatchEvent(*event);
+}
+
 }  // namespace blink

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

@@ -20,8 +20,12 @@ class CORE_EXPORT ApplicationCacheHostForFrame : public ApplicationCacheHost {
       scoped_refptr<base::SingleThreadTaskRunner> task_runner);
 
   // ApplicationCacheHost:
+  void DetachFromDocumentLoader() override;
   bool Update() override;
   bool SwapCache() override;
+  void SetApplicationCache(ApplicationCache*) override;
+  void StopDeferringEvents() override;
+
   void LogMessage(mojom::blink::ConsoleMessageLevel log_level,
                   const String& message) override;
 
@@ -40,6 +44,48 @@ class CORE_EXPORT ApplicationCacheHostForFrame : public ApplicationCacheHost {
  private:
   enum IsNewMasterEntry { MAYBE_NEW_ENTRY, NEW_ENTRY, OLD_ENTRY };
 
+  struct DeferredEvent {
+    mojom::AppCacheEventID event_id;
+    int progress_total;
+    int progress_done;
+    mojom::AppCacheErrorReason error_reason;
+    String error_url;
+    int error_status;
+    String error_message;
+    DeferredEvent(mojom::AppCacheEventID id,
+                  int progress_total,
+                  int progress_done,
+                  mojom::AppCacheErrorReason error_reason,
+                  const String& error_url,
+                  int error_status,
+                  const String& error_message)
+        : event_id(id),
+          progress_total(progress_total),
+          progress_done(progress_done),
+          error_reason(error_reason),
+          error_url(error_url),
+          error_status(error_status),
+          error_message(error_message) {}
+  };
+
+  void NotifyApplicationCache(mojom::AppCacheEventID,
+                              int progress_total,
+                              int progress_done,
+                              mojom::AppCacheErrorReason,
+                              const String& error_url,
+                              int error_status,
+                              const String& error_message) override;
+
+  void DispatchDOMEvent(mojom::AppCacheEventID,
+                        int progress_total,
+                        int progress_done,
+                        mojom::AppCacheErrorReason,
+                        const String& error_url,
+                        int error_status,
+                        const String& error_message);
+
+  WeakMember<ApplicationCache> dom_application_cache_ = nullptr;
+
   Member<LocalFrame> local_frame_;
   bool is_get_method_ = false;
   bool was_select_cache_called_ = false;
@@ -48,6 +94,10 @@ class CORE_EXPORT ApplicationCacheHostForFrame : public ApplicationCacheHost {
   ResourceResponse document_response_;
   KURL document_url_;
   KURL original_main_resource_url_;  // Used to detect redirection.
+
+  // Events are deferred until after document onload.
+  bool defers_events_ = true;
+  Vector<DeferredEvent> deferred_events_;
 };
 
 }  // namespace blink

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

@@ -32,6 +32,11 @@ bool ApplicationCacheHostForSharedWorker::SwapCache() {
   return false;
 }
 
+void ApplicationCacheHostForSharedWorker::SetApplicationCache(
+    ApplicationCache* dom_application_cache) {
+  NOTREACHED();
+}
+
 void ApplicationCacheHostForSharedWorker::LogMessage(
     mojom::blink::ConsoleMessageLevel log_level,
     const String& message) {}

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

@@ -19,6 +19,7 @@ class ApplicationCacheHostForSharedWorker final : public ApplicationCacheHost {
   // ApplicationCacheHost:
   bool Update() override;
   bool SwapCache() override;
+  void SetApplicationCache(ApplicationCache*) override;
   void LogMessage(mojom::blink::ConsoleMessageLevel log_level,
                   const String& message) override;
   void SetSubresourceFactory(