Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: always use new site instance for a new navigation #19826

Merged
merged 1 commit into from Aug 19, 2019
Merged

fix: always use new site instance for a new navigation #19826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions atom/browser/atom_browser_client.cc
View file
Open in desktop
Expand Up @@ -426,6 +426,7 @@ AtomBrowserClient::ShouldOverrideSiteInstanceForNavigation(
content::RenderFrameHost* speculative_rfh,
content::BrowserContext* browser_context,
const GURL& url,
bool has_navigation_started,
bool has_response_started,
content::SiteInstance** affinity_site_instance) const {
if (g_suppress_renderer_process_restart) {
Expand Down Expand Up @@ -460,6 +461,13 @@ AtomBrowserClient::ShouldOverrideSiteInstanceForNavigation(
return SiteInstanceForNavigationType::FORCE_CURRENT;
}

if (!has_navigation_started) {
// If the navigation didn't start yet, ignore any candidate site instance.
// If such instance exists, it belongs to a previous navigation still
// taking place. Fixes https://github.com/electron/electron/issues/17576.
return SiteInstanceForNavigationType::FORCE_NEW;
}

return SiteInstanceForNavigationType::FORCE_CANDIDATE_OR_NEW;
}

Expand Down
1 change: 1 addition & 0 deletions atom/browser/atom_browser_client.h
View file
Open in desktop
Expand Up @@ -84,6 +84,7 @@ class AtomBrowserClient : public content::ContentBrowserClient,
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 override;
void RegisterPendingSiteInstance(
Expand Down
35 changes: 23 additions & 12 deletions patches/common/chromium/frame_host_manager.patch
View file
Open in desktop
Expand Up @@ -42,7 +42,7 @@ index 775b64a8d20f89845812852a2904a1e6875c2b4a..5235b57bbf44fc7b30ca6943c43a290f
// another SiteInstance for the same site.
void RegisterSiteInstance(SiteInstanceImpl* site_instance);
diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc
index 297b61198dd46114b3d8c89488a71ed01aa299c4..40b848a8b448bed2d167bf5f6c0f25971b603ed2 100644
index 297b61198dd46114b3d8c89488a71ed01aa299c4..6adf8ef7c00cabff4cd3c3ad6af01125c24791c7 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -2127,6 +2127,20 @@ bool RenderFrameHostManager::InitRenderView(
Expand All @@ -66,24 +66,26 @@ index 297b61198dd46114b3d8c89488a71ed01aa299c4..40b848a8b448bed2d167bf5f6c0f2597
// First, check if the navigation can switch SiteInstances. If not, the
// navigation should use the current SiteInstance.
SiteInstance* current_site_instance = render_frame_host_->GetSiteInstance();
@@ -2158,6 +2172,53 @@ RenderFrameHostManager::GetSiteInstanceForNavigationRequest(
@@ -2158,6 +2172,59 @@ RenderFrameHostManager::GetSiteInstanceForNavigationRequest(
request.common_params().url);
no_renderer_swap_allowed |=
request.from_begin_navigation() && !can_renderer_initiate_transfer;
+
+ if (!GetContentClient()->browser()->CanUseCustomSiteInstance()) {
+ bool has_navigation_started = request.state() != NavigationRequest::NOT_STARTED;
+ bool has_response_started =
+ (request.state() == NavigationRequest::RESPONSE_STARTED ||
+ request.state() == NavigationRequest::FAILED) &&
+ !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_response_started,
+ &affinity_site_instance);
+ request.common_params().url, has_navigation_started,
+ has_response_started, &affinity_site_instance);
+ switch (siteInstanceType) {
+ case ContentBrowserClient::SiteInstanceForNavigationType::
+ FORCE_CANDIDATE_OR_NEW:
Expand All @@ -92,6 +94,12 @@ index 297b61198dd46114b3d8c89488a71ed01aa299c4..40b848a8b448bed2d167bf5f6c0f2597
+ ? 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();
Expand All @@ -108,9 +116,7 @@ index 297b61198dd46114b3d8c89488a71ed01aa299c4..40b848a8b448bed2d167bf5f6c0f2597
+ break;
+ }
+ if (overriden_site_instance) {
+ if (siteInstanceType ==
+ ContentBrowserClient::SiteInstanceForNavigationType::
+ FORCE_CANDIDATE_OR_NEW) {
+ if (should_register_site_instance) {
+ GetContentClient()->browser()->RegisterPendingSiteInstance(
+ render_frame_host_.get(), overriden_site_instance.get());
+ }
Expand All @@ -120,7 +126,7 @@ index 297b61198dd46114b3d8c89488a71ed01aa299c4..40b848a8b448bed2d167bf5f6c0f2597
} else {
// Subframe navigations will use the current renderer, unless specifically
// allowed to swap processes.
@@ -2169,23 +2230,28 @@ RenderFrameHostManager::GetSiteInstanceForNavigationRequest(
@@ -2169,23 +2236,28 @@ RenderFrameHostManager::GetSiteInstanceForNavigationRequest(
if (no_renderer_swap_allowed && !should_swap_for_error_isolation)
return scoped_refptr<SiteInstance>(current_site_instance);

Expand Down Expand Up @@ -179,10 +185,10 @@ index a46901055bdf17b6b0dab14edf753b234dc04a12..29c201b0c95eb0c7a35f47d6f3ab5b48
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 e51c018b95b0f2dff650a83b4af638ef431aaa11..be9728a49ead7c2667a7fbe2e26b10ab6c694b6d 100644
index e51c018b95b0f2dff650a83b4af638ef431aaa11..b5099cba2f5a09d04dd40cb3be4d87895c6980e6 100644
--- a/content/public/browser/content_browser_client.cc
+++ b/content/public/browser/content_browser_client.cc
@@ -52,6 +52,20 @@ void OverrideOnBindInterface(const service_manager::BindSourceInfo& remote_info,
@@ -52,6 +52,21 @@ void OverrideOnBindInterface(const service_manager::BindSourceInfo& remote_info,
handle);
}

Expand All @@ -195,6 +201,7 @@ index e51c018b95b0f2dff650a83b4af638ef431aaa11..be9728a49ead7c2667a7fbe2e26b10ab
+ 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;
Expand All @@ -204,10 +211,10 @@ index e51c018b95b0f2dff650a83b4af638ef431aaa11..be9728a49ead7c2667a7fbe2e26b10ab
const MainFunctionParams& parameters) {
return nullptr;
diff --git a/content/public/browser/content_browser_client.h b/content/public/browser/content_browser_client.h
index f8e52b835c556ccb9526f1df644eb367152af75e..1837b06d3d5cf1ce26f391222a4e06560b8363f1 100644
index f8e52b835c556ccb9526f1df644eb367152af75e..c040a866285adb570fd3f809ef602612706fc332 100644
--- a/content/public/browser/content_browser_client.h
+++ b/content/public/browser/content_browser_client.h
@@ -211,8 +211,40 @@ CONTENT_EXPORT void OverrideOnBindInterface(
@@ -211,8 +211,44 @@ CONTENT_EXPORT void OverrideOnBindInterface(
// the observer interfaces.)
class CONTENT_EXPORT ContentBrowserClient {
public:
Expand All @@ -220,6 +227,9 @@ index f8e52b835c556ccb9526f1df644eb367152af75e..1837b06d3d5cf1ce26f391222a4e0656
+ // 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,
+
Expand All @@ -237,6 +247,7 @@ index f8e52b835c556ccb9526f1df644eb367152af75e..1837b06d3d5cf1ce26f391222a4e0656
+ 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;
+
Expand Down