chromium/chromium-84-revert-manage-ManifestManagerHost-per-document.patch

commit fce18322d66ea6e67275e13242dae2a8c06d3ae2
Author: Yuzu Saijo <yuzus@chromium.org>
Date:   Thu May 14 05:02:09 2020 +0000

    [content] Manage ManifestManagerHost per-document
    
    This CL converts ManifestManagerHost class away from being owned by
    WebContents and makes it a part of RenderDocumentHostUserData.
    
    We used to create an instance per-WebContents, but now it is created for
    document, and only for main-frame.
    
    Bug: 1077170
    Change-Id: I2a185a6cd6f290d3b904d359b55638bf09eaa79f
    Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2147485
    Commit-Queue: Yuzu Saijo <yuzus@chromium.org>
    Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
    Reviewed-by: Kentaro Hara <haraken@chromium.org>
    Reviewed-by: Sreeja Kamishetty <sreejakshetty@chromium.org>
    Reviewed-by: Alexander Timin <altimin@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#768647}

diff --git b/content/browser/devtools/protocol/page_handler.cc a/content/browser/devtools/protocol/page_handler.cc
index b1821434b975..929b63ab875e 100644
--- b/content/browser/devtools/protocol/page_handler.cc
+++ a/content/browser/devtools/protocol/page_handler.cc
@@ -961,14 +961,14 @@ Response PageHandler::SetDownloadBehavior(const std::string& behavior,
 
 void PageHandler::GetAppManifest(
     std::unique_ptr<GetAppManifestCallback> callback) {
-  if (!host_) {
+  WebContentsImpl* web_contents = GetWebContents();
+  if (!web_contents || !web_contents->GetManifestManagerHost()) {
     callback->sendFailure(Response::ServerError("Cannot retrieve manifest"));
     return;
   }
-  ManifestManagerHost::GetOrCreateForCurrentDocument(host_->GetMainFrame())
-      ->RequestManifestDebugInfo(base::BindOnce(&PageHandler::GotManifest,
-                                                weak_factory_.GetWeakPtr(),
-                                                std::move(callback)));
+  web_contents->GetManifestManagerHost()->RequestManifestDebugInfo(
+      base::BindOnce(&PageHandler::GotManifest, weak_factory_.GetWeakPtr(),
+                     std::move(callback)));
 }
 
 WebContentsImpl* PageHandler::GetWebContents() {
diff --git b/content/browser/frame_host/render_document_host_user_data_browsertest.cc a/content/browser/frame_host/render_document_host_user_data_browsertest.cc
index 09dff7842517..290e5509b448 100644
--- b/content/browser/frame_host/render_document_host_user_data_browsertest.cc
+++ a/content/browser/frame_host/render_document_host_user_data_browsertest.cc
@@ -88,7 +88,7 @@ class RenderDocumentHostUserDataTest : public ContentBrowserTest {
 
 // Test basic functionality of RenderDocumentHostUserData.
 IN_PROC_BROWSER_TEST_F(RenderDocumentHostUserDataTest,
-                       GetCreateAndDeleteForCurrentDocument) {
+                       GetAndCreateForCurrentDocument) {
   ASSERT_TRUE(embedded_test_server()->Start());
   GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
 
@@ -104,14 +104,8 @@ IN_PROC_BROWSER_TEST_F(RenderDocumentHostUserDataTest,
   // 3) Create Data and check that GetForCurrentDocument shouldn't return null
   // now.
   Data::CreateForCurrentDocument(rfh_a);
-  base::WeakPtr<Data> created_data =
-      Data::GetForCurrentDocument(rfh_a)->GetWeakPtr();
-  EXPECT_TRUE(created_data);
-
-  // 4) Delete Data and check that GetForCurrentDocument should return null.
-  Data::DeleteForCurrentDocument(rfh_a);
-  EXPECT_FALSE(created_data);
-  EXPECT_FALSE(Data::GetForCurrentDocument(rfh_a));
+  data = Data::GetForCurrentDocument(rfh_a);
+  EXPECT_TRUE(data);
 }
 
 // Tests that RenderDocumentHostUserData objects are different for each
diff --git b/content/browser/frame_host/render_frame_host_impl.cc a/content/browser/frame_host/render_frame_host_impl.cc
index 30bc648d74ef..d10d99df7f1f 100644
--- b/content/browser/frame_host/render_frame_host_impl.cc
+++ a/content/browser/frame_host/render_frame_host_impl.cc
@@ -75,7 +75,6 @@
 #include "content/browser/loader/navigation_url_loader_impl.h"
 #include "content/browser/loader/prefetch_url_loader_service.h"
 #include "content/browser/log_console_message.h"
-#include "content/browser/manifest/manifest_manager_host.h"
 #include "content/browser/media/capture/audio_mirroring_manager.h"
 #include "content/browser/media/media_interface_proxy.h"
 #include "content/browser/media/webaudio/audio_context_manager_impl.h"
@@ -6146,15 +6145,6 @@ void RenderFrameHostImpl::SetUpMojoIfNeeded() {
               std::make_unique<ActiveURLMessageFilter>(impl));
         },
         base::Unretained(this)));
-
-    associated_registry_->AddInterface(base::BindRepeating(
-        [](RenderFrameHostImpl* impl,
-           mojo::PendingAssociatedReceiver<
-               blink::mojom::ManifestUrlChangeObserver> receiver) {
-          ManifestManagerHost::GetOrCreateForCurrentDocument(impl)
-              ->BindObserver(std::move(receiver));
-        },
-        base::Unretained(this)));
   }
 
   associated_registry_->AddInterface(base::BindRepeating(
diff --git b/content/browser/frame_host/render_frame_host_impl.h a/content/browser/frame_host/render_frame_host_impl.h
index bfd421386795..2bba134e0dc0 100644
--- b/content/browser/frame_host/render_frame_host_impl.h
+++ a/content/browser/frame_host/render_frame_host_impl.h
@@ -1595,10 +1595,6 @@ class CONTENT_EXPORT RenderFrameHostImpl
     document_associated_data_.SetUserData(key, std::move(data));
   }
 
-  void RemoveRenderDocumentHostUserData(const void* key) {
-    document_associated_data_.RemoveUserData(key);
-  }
-
   // Returns the child RenderFrameHostImpl if |child_frame_routing_id| is an
   // immediate child of this FrameTreeNode. |child_frame_routing_id| is
   // considered untrusted, so the renderer process is killed if it refers to a
diff --git b/content/browser/manifest/manifest_manager_host.cc a/content/browser/manifest/manifest_manager_host.cc
index 68ea016c62eb..b063e0d1e98e 100644
--- b/content/browser/manifest/manifest_manager_host.cc
+++ a/content/browser/manifest/manifest_manager_host.cc
@@ -9,34 +9,25 @@
 #include "base/bind.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/web_contents.h"
 #include "services/service_manager/public/cpp/interface_provider.h"
 #include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
 #include "third_party/blink/public/common/manifest/manifest.h"
 
 namespace content {
 
-ManifestManagerHost::ManifestManagerHost(RenderFrameHost* render_frame_host)
-    : manifest_manager_frame_(render_frame_host) {
-  // Check that |manifest_manager_frame_| is a main frame.
-  DCHECK(!manifest_manager_frame_->GetParent());
-}
+ManifestManagerHost::ManifestManagerHost(WebContents* web_contents)
+    : WebContentsObserver(web_contents),
+      manifest_url_change_observer_receivers_(web_contents, this) {}
 
 ManifestManagerHost::~ManifestManagerHost() {
   OnConnectionError();
 }
 
-void ManifestManagerHost::BindObserver(
-    mojo::PendingAssociatedReceiver<blink::mojom::ManifestUrlChangeObserver>
-        receiver) {
-  manifest_url_change_observer_receiver_.Bind(std::move(receiver));
-}
-
-ManifestManagerHost* ManifestManagerHost::GetOrCreateForCurrentDocument(
-    RenderFrameHostImpl* rfh) {
-  DCHECK(rfh->is_main_frame());
-  if (!GetForCurrentDocument(rfh))
-    CreateForCurrentDocument(rfh);
-  return GetForCurrentDocument(rfh);
+void ManifestManagerHost::RenderFrameDeleted(
+    RenderFrameHost* render_frame_host) {
+  if (render_frame_host == manifest_manager_frame_)
+    OnConnectionError();
 }
 
 void ManifestManagerHost::GetManifest(GetManifestCallback callback) {
@@ -54,7 +45,11 @@ void ManifestManagerHost::RequestManifestDebugInfo(
 }
 
 blink::mojom::ManifestManager& ManifestManagerHost::GetManifestManager() {
+  if (manifest_manager_frame_ != web_contents()->GetMainFrame())
+    OnConnectionError();
+
   if (!manifest_manager_) {
+    manifest_manager_frame_ = web_contents()->GetMainFrame();
     manifest_manager_frame_->GetRemoteInterfaces()->GetInterface(
         manifest_manager_.BindNewPipeAndPassReceiver());
     manifest_manager_.set_disconnect_handler(base::BindOnce(
@@ -64,6 +59,8 @@ blink::mojom::ManifestManager& ManifestManagerHost::GetManifestManager() {
 }
 
 void ManifestManagerHost::OnConnectionError() {
+  manifest_manager_frame_ = nullptr;
+  manifest_manager_.reset();
   std::vector<GetManifestCallback> callbacks;
   for (CallbackMap::iterator it(&callbacks_); !it.IsAtEnd(); it.Advance()) {
     callbacks.push_back(std::move(*it.GetCurrentValue()));
@@ -71,10 +68,6 @@ void ManifestManagerHost::OnConnectionError() {
   callbacks_.Clear();
   for (auto& callback : callbacks)
     std::move(callback).Run(GURL(), blink::Manifest());
-
-  if (GetForCurrentDocument(manifest_manager_frame_)) {
-    DeleteForCurrentDocument(manifest_manager_frame_);
-  }
 }
 
 void ManifestManagerHost::OnRequestManifestResponse(
@@ -88,16 +81,12 @@ void ManifestManagerHost::OnRequestManifestResponse(
 
 void ManifestManagerHost::ManifestUrlChanged(
     const base::Optional<GURL>& manifest_url) {
-  if (!manifest_manager_frame_->IsCurrent())
+  if (manifest_url_change_observer_receivers_.GetCurrentTargetFrame() !=
+      web_contents()->GetMainFrame()) {
     return;
-
-  // TODO(yuzus): |NotifyManifestUrlChanged| should start taking a
-  // |RenderFrameHost| parameter.
-  WebContents* web_contents =
-      WebContents::FromRenderFrameHost(manifest_manager_frame_);
-  static_cast<WebContentsImpl*>(web_contents)
+  }
+  static_cast<WebContentsImpl*>(web_contents())
       ->NotifyManifestUrlChanged(manifest_url);
 }
 
-RENDER_DOCUMENT_HOST_USER_DATA_KEY_IMPL(ManifestManagerHost)
 }  // namespace content
diff --git b/content/browser/manifest/manifest_manager_host.h a/content/browser/manifest/manifest_manager_host.h
index 57f51dc9fad7..3dc0bbf6e1ad 100644
--- b/content/browser/manifest/manifest_manager_host.h
+++ a/content/browser/manifest/manifest_manager_host.h
@@ -8,8 +8,8 @@
 #include "base/callback_forward.h"
 #include "base/containers/id_map.h"
 #include "base/macros.h"
-#include "content/public/browser/render_document_host_user_data.h"
-#include "mojo/public/cpp/bindings/associated_receiver.h"
+#include "content/public/browser/web_contents_observer.h"
+#include "content/public/browser/web_contents_receiver_set.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "third_party/blink/public/mojom/manifest/manifest_manager.mojom.h"
 #include "third_party/blink/public/mojom/manifest/manifest_observer.mojom.h"
@@ -21,16 +21,16 @@ struct Manifest;
 namespace content {
 
 class RenderFrameHost;
-class RenderFrameHostImpl;
+class WebContents;
 
 // ManifestManagerHost is a helper class that allows callers to get the Manifest
 // associated with the main frame of the observed WebContents. It handles the
 // IPC messaging with the child process.
 // TODO(mlamouri): keep a cached version and a dirty bit here.
-class ManifestManagerHost
-    : public RenderDocumentHostUserData<ManifestManagerHost>,
-      public blink::mojom::ManifestUrlChangeObserver {
+class ManifestManagerHost : public WebContentsObserver,
+                            public blink::mojom::ManifestUrlChangeObserver {
  public:
+  explicit ManifestManagerHost(WebContents* web_contents);
   ~ManifestManagerHost() override;
 
   using GetManifestCallback =
@@ -44,18 +44,10 @@ class ManifestManagerHost
   void RequestManifestDebugInfo(
       blink::mojom::ManifestManager::RequestManifestDebugInfoCallback callback);
 
-  void BindObserver(
-      mojo::PendingAssociatedReceiver<blink::mojom::ManifestUrlChangeObserver>
-          receiver);
-
-  static ManifestManagerHost* GetOrCreateForCurrentDocument(
-      RenderFrameHostImpl* rfh);
+  // WebContentsObserver
+  void RenderFrameDeleted(RenderFrameHost* render_frame_host) override;
 
  private:
-  explicit ManifestManagerHost(RenderFrameHost* render_frame_host);
-
-  friend class RenderDocumentHostUserData<ManifestManagerHost>;
-
   using CallbackMap = base::IDMap<std::unique_ptr<GetManifestCallback>>;
 
   blink::mojom::ManifestManager& GetManifestManager();
@@ -68,14 +60,13 @@ class ManifestManagerHost
   // blink::mojom::ManifestUrlChangeObserver:
   void ManifestUrlChanged(const base::Optional<GURL>& manifest_url) override;
 
-  RenderFrameHost* manifest_manager_frame_;
+  RenderFrameHost* manifest_manager_frame_ = nullptr;
   mojo::Remote<blink::mojom::ManifestManager> manifest_manager_;
   CallbackMap callbacks_;
 
-  mojo::AssociatedReceiver<blink::mojom::ManifestUrlChangeObserver>
-      manifest_url_change_observer_receiver_{this};
+  WebContentsFrameReceiverSet<blink::mojom::ManifestUrlChangeObserver>
+      manifest_url_change_observer_receivers_;
 
-  RENDER_DOCUMENT_HOST_USER_DATA_KEY_DECL();
   DISALLOW_COPY_AND_ASSIGN(ManifestManagerHost);
 };
 
diff --git b/content/browser/web_contents/web_contents_impl.cc a/content/browser/web_contents/web_contents_impl.cc
index 024415bb096e..115f480ce8c1 100644
--- b/content/browser/web_contents/web_contents_impl.cc
+++ a/content/browser/web_contents/web_contents_impl.cc
@@ -2122,6 +2122,8 @@ void WebContentsImpl::Init(const WebContents::CreateParams& params) {
 
   screen_orientation_provider_.reset(new ScreenOrientationProvider(this));
 
+  manifest_manager_host_.reset(new ManifestManagerHost(this));
+
 #if defined(OS_ANDROID)
   DateTimeChooserAndroid::CreateForWebContents(this);
 #endif
@@ -4202,10 +4204,7 @@ bool WebContentsImpl::WasEverAudible() {
 }
 
 void WebContentsImpl::GetManifest(GetManifestCallback callback) {
-  // TODO(yuzus, 1061899): Move this function to RenderFrameHostImpl.
-  ManifestManagerHost* manifest_manager_host =
-      ManifestManagerHost::GetOrCreateForCurrentDocument(GetMainFrame());
-  manifest_manager_host->GetManifest(std::move(callback));
+  manifest_manager_host_->GetManifest(std::move(callback));
 }
 
 void WebContentsImpl::ExitFullscreen(bool will_cause_resize) {
diff --git b/content/browser/web_contents/web_contents_impl.h a/content/browser/web_contents/web_contents_impl.h
index 59510b1f8744..e86be96fc23b 100644
--- b/content/browser/web_contents/web_contents_impl.h
+++ a/content/browser/web_contents/web_contents_impl.h
@@ -102,6 +102,7 @@ class DisplayCutoutHostImpl;
 class FindRequestManager;
 class JavaScriptDialogManager;
 class JavaScriptDialogNavigationDeferrer;
+class ManifestManagerHost;
 class MediaWebContentsObserver;
 class NFCHost;
 class PluginContentOriginAllowlist;
@@ -311,6 +312,10 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
 
   void NotifyManifestUrlChanged(const base::Optional<GURL>& manifest_url);
 
+  ManifestManagerHost* GetManifestManagerHost() const {
+    return manifest_manager_host_.get();
+  }
+
 #if defined(OS_ANDROID)
   void SetMainFrameImportance(ChildProcessImportance importance);
 #endif
@@ -1897,6 +1902,8 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
 
   std::unique_ptr<ScreenOrientationProvider> screen_orientation_provider_;
 
+  std::unique_ptr<ManifestManagerHost> manifest_manager_host_;
+
   // The accessibility mode for all frames. This is queried when each frame
   // is created, and broadcast to all frames when it changes.
   ui::AXMode accessibility_mode_;
diff --git b/content/public/browser/render_document_host_user_data.cc a/content/public/browser/render_document_host_user_data.cc
index 3b58bf8a3c5e..b1b385455e61 100644
--- b/content/public/browser/render_document_host_user_data.cc
+++ a/content/public/browser/render_document_host_user_data.cc
@@ -23,8 +23,4 @@ void SetRenderDocumentHostUserData(
       key, std::move(data));
 }
 
-void RemoveRenderDocumentHostUserData(RenderFrameHost* rfh, const void* key) {
-  static_cast<RenderFrameHostImpl*>(rfh)->RemoveRenderDocumentHostUserData(key);
-}
-
 }  // namespace content
diff --git b/content/public/browser/render_document_host_user_data.h a/content/public/browser/render_document_host_user_data.h
index a138fd60aa2a..f55f24f60992 100644
--- b/content/public/browser/render_document_host_user_data.h
+++ a/content/public/browser/render_document_host_user_data.h
@@ -22,9 +22,6 @@ CONTENT_EXPORT void SetRenderDocumentHostUserData(
     const void* key,
     std::unique_ptr<base::SupportsUserData::Data> data);
 
-CONTENT_EXPORT void RemoveRenderDocumentHostUserData(RenderFrameHost* rfh,
-                                                     const void* key);
-
 // This class approximates the lifetime of a single blink::Document in the
 // browser process. At the moment RenderFrameHost can correspond to multiple
 // blink::Documents (when RenderFrameHost is reused for same-process
@@ -85,12 +82,6 @@ class RenderDocumentHostUserData : public base::SupportsUserData::Data {
     return static_cast<T*>(GetRenderDocumentHostUserData(rfh, UserDataKey()));
   }
 
-  static void DeleteForCurrentDocument(RenderFrameHost* rfh) {
-    DCHECK(rfh);
-    DCHECK(GetForCurrentDocument(rfh));
-    RemoveRenderDocumentHostUserData(rfh, UserDataKey());
-  }
-
   static const void* UserDataKey() { return &T::kUserDataKey; }
 };