Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
openSUSE
chromium.13679
chromium-84-revert-manage-ManifestManagerHost-p...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File chromium-84-revert-manage-ManifestManagerHost-per-document.patch of Package chromium.13679
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; } };
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
