refactor: clean up unused parts of the render_frame_host patch file (#28769)
* chore: remove CanUseCustomSiteInstance * chore: remove unused ShouldOverrideSiteInstanceForNavigation * chore: remove CreateRelatedSiteInstance * chore: remove BrowsingInstance::CreateSiteInstanceForURL * chore: remove NavigationState::REUSE_SITE_INSTANCE * chore: remove accidentally duplicated code post-patch-clean * chore: reword patch description for new feature-set * chore: remove ShouldForceNewSiteInstance and all its unused helpers
This commit is contained in:
Samuel Attard
GitHub
parent
43d27cc4d1
commit
28904714ae
3 changed files with 5 additions and 458 deletions
|
|
@ -3,195 +3,13 @@ From: deepak1556 <hop2deep@gmail.com>
|
|||
Date: Mon, 3 Jun 2019 14:20:05 -0700
|
||||
Subject: frame_host_manager.patch
|
||||
|
||||
Allows embedder to intercept site instances chosen by chromium
|
||||
and respond with custom instance. Also allows for us to at-runtime
|
||||
enable or disable this patch.
|
||||
Allows embedder to intercept site instances created by chromium.
|
||||
|
||||
diff --git a/content/browser/browsing_instance.cc b/content/browser/browsing_instance.cc
|
||||
index 30e374aeba7aceacdb58add9c79cb46c6b93af4d..3e34643c4e1d5561c6c7b01e3de8cdd46910afb0 100644
|
||||
--- a/content/browser/browsing_instance.cc
|
||||
+++ b/content/browser/browsing_instance.cc
|
||||
@@ -61,6 +61,13 @@ scoped_refptr<SiteInstanceImpl> BrowsingInstance::GetSiteInstanceForURL(
|
||||
return instance;
|
||||
}
|
||||
|
||||
+scoped_refptr<SiteInstanceImpl> BrowsingInstance::CreateSiteInstanceForURL(
|
||||
+ const GURL& url) {
|
||||
+ scoped_refptr<SiteInstanceImpl> instance = new SiteInstanceImpl(this);
|
||||
+ instance->SetSite(UrlInfo(url, UrlInfo::OriginIsolationRequest::kNone));
|
||||
+ return instance;
|
||||
+}
|
||||
+
|
||||
SiteInfo BrowsingInstance::GetSiteInfoForURL(const UrlInfo& url_info,
|
||||
bool allow_default_instance) {
|
||||
scoped_refptr<SiteInstanceImpl> site_instance =
|
||||
diff --git a/content/browser/browsing_instance.h b/content/browser/browsing_instance.h
|
||||
index f426d1ebdaa9d13c7e47f22ce617e2b61a862854..26907541dd4268e3f8c82676bdda892bd1ed04e7 100644
|
||||
--- a/content/browser/browsing_instance.h
|
||||
+++ b/content/browser/browsing_instance.h
|
||||
@@ -151,6 +151,11 @@ class CONTENT_EXPORT BrowsingInstance final
|
||||
const UrlInfo& url_info,
|
||||
bool allow_default_instance);
|
||||
|
||||
+ // Create a new SiteInstance for the given URL bound the current
|
||||
+ // BrowsingInstance.
|
||||
+ scoped_refptr<SiteInstanceImpl> CreateSiteInstanceForURL(
|
||||
+ const GURL& url);
|
||||
+
|
||||
// Adds the given SiteInstance to our map, to ensure that we do not create
|
||||
// another SiteInstance for the same site.
|
||||
void RegisterSiteInstance(SiteInstanceImpl* site_instance);
|
||||
diff --git a/content/browser/renderer_host/navigation_request.cc b/content/browser/renderer_host/navigation_request.cc
|
||||
index 1f0b01cbf17267bd8c03deb4a28938566f7033ef..5ccf84c2b847e9d2cbc97f4b2390220324a807c3 100644
|
||||
--- a/content/browser/renderer_host/navigation_request.cc
|
||||
+++ b/content/browser/renderer_host/navigation_request.cc
|
||||
@@ -1572,6 +1572,21 @@ void NavigationRequest::BeginNavigation() {
|
||||
if (IsSameDocument()) {
|
||||
render_frame_host_ = frame_tree_node_->current_frame_host();
|
||||
} else {
|
||||
+ // In Electron, a new process is started for every navigation when node is
|
||||
+ // integrated in the renderer. Since GetFrameHostForNavigation is called more
|
||||
+ // than once per navigation, we want to return the same frame host once
|
||||
+ // the response has started, otherwise if the frame host that started the response
|
||||
+ // and the frame host that is available for commit don't match then request will
|
||||
+ // be cancelled. We rely on the NavigationRequest::state_ to determine this
|
||||
+ // factor
|
||||
+ //
|
||||
+ // bool has_response_started =
|
||||
+ // (request->state() >= NavigationRequest::WILL_PROCESS_RESPONSE &&
|
||||
+ // !speculative_render_frame_host_);
|
||||
+ //
|
||||
+ // Hence this call is necesary before the call to GetFrameHostForNavigation.
|
||||
+ SetState(REUSE_SITE_INSTANCE);
|
||||
+
|
||||
// Select an appropriate RenderFrameHost.
|
||||
std::string frame_host_choice_reason;
|
||||
render_frame_host_ =
|
||||
@@ -6152,6 +6167,7 @@ void NavigationRequest::CheckStateTransition(NavigationState state) const {
|
||||
{WILL_START_REQUEST, {
|
||||
WILL_REDIRECT_REQUEST,
|
||||
WILL_PROCESS_RESPONSE,
|
||||
+ REUSE_SITE_INSTANCE,
|
||||
READY_TO_COMMIT,
|
||||
DID_COMMIT,
|
||||
CANCELING,
|
||||
@@ -6165,10 +6181,15 @@ void NavigationRequest::CheckStateTransition(NavigationState state) const {
|
||||
WILL_FAIL_REQUEST,
|
||||
}},
|
||||
{WILL_PROCESS_RESPONSE, {
|
||||
+ REUSE_SITE_INSTANCE,
|
||||
READY_TO_COMMIT,
|
||||
CANCELING,
|
||||
WILL_FAIL_REQUEST,
|
||||
}},
|
||||
+ {REUSE_SITE_INSTANCE, {
|
||||
+ WILL_PROCESS_RESPONSE,
|
||||
+ READY_TO_COMMIT,
|
||||
+ }},
|
||||
{READY_TO_COMMIT, {
|
||||
NOT_STARTED,
|
||||
DID_COMMIT,
|
||||
diff --git a/content/browser/renderer_host/navigation_request.h b/content/browser/renderer_host/navigation_request.h
|
||||
index 304de734c34efd3c4f6ab561d66ecf7d655387f8..6817bfd0ea8934b2b1a1d62415b5475d5c88d20b 100644
|
||||
--- a/content/browser/renderer_host/navigation_request.h
|
||||
+++ b/content/browser/renderer_host/navigation_request.h
|
||||
@@ -140,6 +140,10 @@ class CONTENT_EXPORT NavigationRequest
|
||||
// asynchronous.
|
||||
WILL_PROCESS_RESPONSE,
|
||||
|
||||
+ // Electron: state that is between than WILL_PROCESS_RESPONSE and
|
||||
+ // READY_TO_COMMIT to force reuse the same site instance.
|
||||
+ REUSE_SITE_INSTANCE,
|
||||
+
|
||||
// The response started on the IO thread and is ready to be committed.
|
||||
READY_TO_COMMIT,
|
||||
|
||||
diff --git a/content/browser/renderer_host/render_frame_host_manager.cc b/content/browser/renderer_host/render_frame_host_manager.cc
|
||||
index af497e6916d72bfbd40fe9f131be6022482ef810..74f9fe3a0a0de20d48bc8b2d397109381ea66481 100644
|
||||
index af497e6916d72bfbd40fe9f131be6022482ef810..a5411ead9c465dab2814ebb25836d10e2e8cd494 100644
|
||||
--- a/content/browser/renderer_host/render_frame_host_manager.cc
|
||||
+++ b/content/browser/renderer_host/render_frame_host_manager.cc
|
||||
@@ -2859,6 +2859,16 @@ scoped_refptr<SiteInstance>
|
||||
RenderFrameHostManager::GetSiteInstanceForNavigationRequest(
|
||||
NavigationRequest* request,
|
||||
std::string* reason) {
|
||||
+ // Compute the SiteInstance that the navigation should use, which will be
|
||||
+ // either the current SiteInstance or a new one.
|
||||
+ //
|
||||
+ // TODO(clamy): We should also consider as a candidate SiteInstance the
|
||||
+ // speculative SiteInstance that was computed on redirects.
|
||||
+ SiteInstanceImpl* candidate_site_instance =
|
||||
+ speculative_render_frame_host_
|
||||
+ ? speculative_render_frame_host_->GetSiteInstance()
|
||||
+ : nullptr;
|
||||
+
|
||||
SiteInstance* current_site_instance = render_frame_host_->GetSiteInstance();
|
||||
|
||||
// All children of MHTML documents must be MHTML documents. They all live in
|
||||
@@ -2886,10 +2896,59 @@ RenderFrameHostManager::GetSiteInstanceForNavigationRequest(
|
||||
//
|
||||
// TODO(clamy): We should also consider as a candidate SiteInstance the
|
||||
// speculative SiteInstance that was computed on redirects.
|
||||
- SiteInstanceImpl* candidate_site_instance =
|
||||
- speculative_render_frame_host_
|
||||
- ? speculative_render_frame_host_->GetSiteInstance()
|
||||
- : nullptr;
|
||||
+ if (!GetContentClient()->browser()->CanUseCustomSiteInstance() &&
|
||||
+ frame_tree_node_->IsMainFrame()) {
|
||||
+ BrowserContext* browser_context = GetNavigationController().GetBrowserContext();
|
||||
+ bool has_navigation_started = request->state() != NavigationRequest::NOT_STARTED;
|
||||
+ bool has_response_started =
|
||||
+ (request->state() >= NavigationRequest::WILL_PROCESS_RESPONSE &&
|
||||
+ !speculative_render_frame_host_);
|
||||
+ // Gives user a chance to choose a custom site instance.
|
||||
+ SiteInstance* affinity_site_instance = nullptr;
|
||||
+ scoped_refptr<SiteInstance> overriden_site_instance;
|
||||
+ bool should_register_site_instance = false;
|
||||
+ ContentBrowserClient::SiteInstanceForNavigationType siteInstanceType =
|
||||
+ GetContentClient()->browser()->ShouldOverrideSiteInstanceForNavigation(
|
||||
+ current_frame_host(), speculative_frame_host(), browser_context,
|
||||
+ request->common_params().url, has_navigation_started,
|
||||
+ has_response_started, &affinity_site_instance);
|
||||
+ switch (siteInstanceType) {
|
||||
+ case ContentBrowserClient::SiteInstanceForNavigationType::
|
||||
+ FORCE_CANDIDATE_OR_NEW:
|
||||
+ overriden_site_instance =
|
||||
+ candidate_site_instance
|
||||
+ ? candidate_site_instance
|
||||
+ : current_site_instance->CreateRelatedSiteInstance(
|
||||
+ request->common_params().url);
|
||||
+ should_register_site_instance = true;
|
||||
+ break;
|
||||
+ case ContentBrowserClient::SiteInstanceForNavigationType::FORCE_NEW:
|
||||
+ overriden_site_instance = current_site_instance->CreateRelatedSiteInstance(
|
||||
+ request->common_params().url);
|
||||
+ should_register_site_instance = true;
|
||||
+ break;
|
||||
+ case ContentBrowserClient::SiteInstanceForNavigationType::FORCE_CURRENT:
|
||||
+ overriden_site_instance = render_frame_host_->GetSiteInstance();
|
||||
+ break;
|
||||
+ case ContentBrowserClient::SiteInstanceForNavigationType::FORCE_AFFINITY:
|
||||
+ DCHECK(affinity_site_instance);
|
||||
+ overriden_site_instance =
|
||||
+ scoped_refptr<SiteInstance>(affinity_site_instance);
|
||||
+ break;
|
||||
+ case ContentBrowserClient::SiteInstanceForNavigationType::ASK_CHROMIUM:
|
||||
+ DCHECK(!affinity_site_instance);
|
||||
+ break;
|
||||
+ default:
|
||||
+ break;
|
||||
+ }
|
||||
+ if (overriden_site_instance) {
|
||||
+ if (should_register_site_instance) {
|
||||
+ GetContentClient()->browser()->RegisterPendingSiteInstance(
|
||||
+ render_frame_host_.get(), overriden_site_instance.get());
|
||||
+ }
|
||||
+ return overriden_site_instance;
|
||||
+ }
|
||||
+ }
|
||||
|
||||
// Account for renderer-initiated reload as well.
|
||||
// Needed as a workaround for https://crbug.com/1045524, remove it when it is
|
||||
@@ -2922,6 +2981,9 @@ RenderFrameHostManager::GetSiteInstanceForNavigationRequest(
|
||||
@@ -2922,6 +2922,9 @@ RenderFrameHostManager::GetSiteInstanceForNavigationRequest(
|
||||
request->ResetStateForSiteInstanceChange();
|
||||
}
|
||||
|
||||
|
|
@ -201,101 +19,14 @@ index af497e6916d72bfbd40fe9f131be6022482ef810..74f9fe3a0a0de20d48bc8b2d39710938
|
|||
return dest_site_instance;
|
||||
}
|
||||
|
||||
diff --git a/content/browser/site_instance_impl.cc b/content/browser/site_instance_impl.cc
|
||||
index 8013a8606e4f01ef63b7b546fd7537f9232648c3..2f74a84667d7cf760642fb1577f22ad85ca00ecd 100644
|
||||
--- a/content/browser/site_instance_impl.cc
|
||||
+++ b/content/browser/site_instance_impl.cc
|
||||
@@ -1135,6 +1135,10 @@ bool SiteInstanceImpl::HasRelatedSiteInstance(const SiteInfo& site_info) {
|
||||
return browsing_instance_->HasSiteInstance(site_info);
|
||||
}
|
||||
|
||||
+scoped_refptr<SiteInstance> SiteInstanceImpl::CreateRelatedSiteInstance(const GURL& url) {
|
||||
+ return browsing_instance_->CreateSiteInstanceForURL(url);
|
||||
+}
|
||||
+
|
||||
scoped_refptr<SiteInstance> SiteInstanceImpl::GetRelatedSiteInstance(
|
||||
const GURL& url) {
|
||||
return GetRelatedSiteInstanceImpl(
|
||||
diff --git a/content/browser/site_instance_impl.h b/content/browser/site_instance_impl.h
|
||||
index 40c314243ae5aa8540ce6528273ee2f0b7f0aca1..dce5cf6cde036d35c1b78b8fa768617628d380c2 100644
|
||||
--- a/content/browser/site_instance_impl.h
|
||||
+++ b/content/browser/site_instance_impl.h
|
||||
@@ -501,6 +501,7 @@ class CONTENT_EXPORT SiteInstanceImpl final : public SiteInstance,
|
||||
BrowserContext* GetBrowserContext() override;
|
||||
const GURL& GetSiteURL() override;
|
||||
scoped_refptr<SiteInstance> GetRelatedSiteInstance(const GURL& url) override;
|
||||
+ scoped_refptr<SiteInstance> CreateRelatedSiteInstance(const GURL& url) override;
|
||||
bool IsRelatedSiteInstance(const SiteInstance* instance) override;
|
||||
size_t GetRelatedActiveContentsCount() override;
|
||||
bool RequiresDedicatedProcess() override;
|
||||
diff --git a/content/public/browser/content_browser_client.cc b/content/public/browser/content_browser_client.cc
|
||||
index 0d2eba4321ece3e9cfb2b16dead343dd51672714..45fe96cb01e4d6e2772f1304eb1c5ac274c4f713 100644
|
||||
--- a/content/public/browser/content_browser_client.cc
|
||||
+++ b/content/public/browser/content_browser_client.cc
|
||||
@@ -72,6 +72,21 @@
|
||||
|
||||
namespace content {
|
||||
|
||||
+bool ContentBrowserClient::CanUseCustomSiteInstance() {
|
||||
+ return false;
|
||||
+}
|
||||
+
|
||||
+ContentBrowserClient::SiteInstanceForNavigationType ContentBrowserClient::ShouldOverrideSiteInstanceForNavigation(
|
||||
+ content::RenderFrameHost* current_rfh,
|
||||
+ content::RenderFrameHost* speculative_rfh,
|
||||
+ content::BrowserContext* browser_context,
|
||||
+ const GURL& url,
|
||||
+ bool has_navigation_started,
|
||||
+ bool has_request_started,
|
||||
+ content::SiteInstance** affinity_site_instance) const {
|
||||
+ return SiteInstanceForNavigationType::ASK_CHROMIUM;
|
||||
+}
|
||||
+
|
||||
std::unique_ptr<BrowserMainParts> ContentBrowserClient::CreateBrowserMainParts(
|
||||
const MainFunctionParams& parameters) {
|
||||
return nullptr;
|
||||
diff --git a/content/public/browser/content_browser_client.h b/content/public/browser/content_browser_client.h
|
||||
index 47befa844ca54f463b627864827aa670454dfed4..010a63958d094f65b0f1838888991ecd6679b086 100644
|
||||
index 47befa844ca54f463b627864827aa670454dfed4..1a27f50e5922c3886816e7523dda98e1d7a87bda 100644
|
||||
--- a/content/public/browser/content_browser_client.h
|
||||
+++ b/content/public/browser/content_browser_client.h
|
||||
@@ -261,8 +261,45 @@ class CONTENT_EXPORT ContentBrowserClient {
|
||||
using IsClipboardPasteContentAllowedCallback =
|
||||
base::OnceCallback<void(ClipboardPasteContentAllowed)>;
|
||||
@@ -263,6 +263,11 @@ class CONTENT_EXPORT ContentBrowserClient {
|
||||
|
||||
+ // Identifies the type of site instance to use for a navigation.
|
||||
+ enum SiteInstanceForNavigationType {
|
||||
+ // Use either the candidate site instance or, if it doesn't exist
|
||||
+ // a new, unrelated site instance for the navigation.
|
||||
+ FORCE_CANDIDATE_OR_NEW = 0,
|
||||
+
|
||||
+ // Use the current site instance for the navigation.
|
||||
+ FORCE_CURRENT,
|
||||
+
|
||||
+ // Use a new, unrelated site instance.
|
||||
+ FORCE_NEW,
|
||||
+
|
||||
+ // Use the provided affinity site instance for the navigation.
|
||||
+ FORCE_AFFINITY,
|
||||
+
|
||||
+ // Delegate the site instance creation to Chromium.
|
||||
+ ASK_CHROMIUM
|
||||
+ };
|
||||
+
|
||||
virtual ~ContentBrowserClient() = default;
|
||||
|
||||
+ // Electron: Allows disabling the below ShouldOverride patch
|
||||
+ virtual bool CanUseCustomSiteInstance();
|
||||
+
|
||||
+ // Electron: Allows overriding the SiteInstance when navigating.
|
||||
+ virtual SiteInstanceForNavigationType ShouldOverrideSiteInstanceForNavigation(
|
||||
+ content::RenderFrameHost* current_rfh,
|
||||
+ content::RenderFrameHost* speculative_rfh,
|
||||
+ content::BrowserContext* browser_context,
|
||||
+ const GURL& url,
|
||||
+ bool has_navigation_started,
|
||||
+ bool has_request_started,
|
||||
+ content::SiteInstance** affinity_site_instance) const;
|
||||
+
|
||||
+ // Electron: Registers a pending site instance during a navigation.
|
||||
+ virtual void RegisterPendingSiteInstance(
|
||||
+ content::RenderFrameHost* rfh,
|
||||
|
|
@ -304,19 +35,3 @@ index 47befa844ca54f463b627864827aa670454dfed4..010a63958d094f65b0f1838888991ecd
|
|||
// Allows the embedder to set any number of custom BrowserMainParts
|
||||
// implementations for the browser startup code. See comments in
|
||||
// browser_main_parts.h.
|
||||
diff --git a/content/public/browser/site_instance.h b/content/public/browser/site_instance.h
|
||||
index 9b90d52cac2b6a0d15e1ec1f562f0e376ce6be7c..5bc69adfab68c45528ba8d5e81b03b3f6db934a7 100644
|
||||
--- a/content/public/browser/site_instance.h
|
||||
+++ b/content/public/browser/site_instance.h
|
||||
@@ -135,6 +135,11 @@ class CONTENT_EXPORT SiteInstance : public base::RefCounted<SiteInstance> {
|
||||
// corresponds to a site URL with the host "example.com".
|
||||
virtual const GURL& GetSiteURL() = 0;
|
||||
|
||||
+ // Create a SiteInstance for the given URL that shares the current
|
||||
+ // BrowsingInstance.
|
||||
+ virtual scoped_refptr<SiteInstance> CreateRelatedSiteInstance(
|
||||
+ const GURL& url) = 0;
|
||||
+
|
||||
// Gets a SiteInstance for the given URL that shares the current
|
||||
// BrowsingInstance, creating a new SiteInstance if necessary. This ensures
|
||||
// that a BrowsingInstance only has one SiteInstance per site, so that pages
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue