Skip to content

Commit

Permalink
feat: add app.allowRendererProcessReuse property to allow runtime dis…
Browse files Browse the repository at this point in the history
…able of site instance overrides

spec: add tests for the new allowRendererProcessReuse property

feat: add console warnings / errors for loading non context-aware native modules
  * Only error if the patch is disabled
  * Warn all the time, this will ship in Electron 7
  • Loading branch information
MarshallOfSound committed May 31, 2019
1 parent c8be05a commit 90af9bc
Show file tree
Hide file tree
Showing 26 changed files with 240 additions and 123 deletions.
2 changes: 1 addition & 1 deletion DEPS
Expand Up @@ -12,7 +12,7 @@ vars = {
'chromium_version':
'ab588d36191964c4bca8de5c320534d95606c861',
'node_version':
'a86a4a160dc520c61a602c949a32a1bc4c0fc633',
'dee0db9864a001ffc16440f725f4952a1a417069',
'nan_version':
'960dd6c70fc9eb136efdf37b4bef18fadbc3436f',

Expand Down
11 changes: 10 additions & 1 deletion atom/browser/api/atom_api_app.cc
Expand Up @@ -1286,6 +1286,13 @@ std::string App::GetUserAgentFallback() {
return AtomBrowserClient::Get()->GetUserAgent();
}

void App::SetBrowserClientCanUseCustomSiteInstance(bool should_disable) {
AtomBrowserClient::Get()->SetCanUseCustomSiteInstance(should_disable);
}
bool App::CanBrowserClientUseCustomSiteInstance() {
return AtomBrowserClient::Get()->CanUseCustomSiteInstance();
}

#if defined(OS_MACOSX)
bool App::MoveToApplicationsFolder(mate::Arguments* args) {
return ui::cocoa::AtomBundleMover::Move(args);
Expand Down Expand Up @@ -1467,7 +1474,9 @@ void App::BuildPrototype(v8::Isolate* isolate,
#endif
.SetProperty("userAgentFallback", &App::GetUserAgentFallback,
&App::SetUserAgentFallback)
.SetMethod("enableSandbox", &App::EnableSandbox);
.SetMethod("enableSandbox", &App::EnableSandbox)
.SetProperty("allowRendererProcessReuse", &App::CanBrowserClientUseCustomSiteInstance,
&App::SetBrowserClientCanUseCustomSiteInstance);
}

} // namespace api
Expand Down
2 changes: 2 additions & 0 deletions atom/browser/api/atom_api_app.h
Expand Up @@ -213,6 +213,8 @@ class App : public AtomBrowserClient::Delegate,
void EnableSandbox(mate::Arguments* args);
void SetUserAgentFallback(const std::string& user_agent);
std::string GetUserAgentFallback();
void SetBrowserClientCanUseCustomSiteInstance(bool should_disable);
bool CanBrowserClientUseCustomSiteInstance();

#if defined(OS_MACOSX)
bool MoveToApplicationsFolder(mate::Arguments* args);
Expand Down
12 changes: 12 additions & 0 deletions atom/browser/atom_browser_client.cc
Expand Up @@ -403,6 +403,14 @@ void AtomBrowserClient::OverrideWebkitPrefs(content::RenderViewHost* host,
web_preferences->OverrideWebkitPrefs(prefs);
}

void AtomBrowserClient::SetCanUseCustomSiteInstance(bool should_disable) {
disable_process_restart_tricks_ = should_disable;
}

bool AtomBrowserClient::CanUseCustomSiteInstance() {
return disable_process_restart_tricks_;
}

content::ContentBrowserClient::SiteInstanceForNavigationType
AtomBrowserClient::ShouldOverrideSiteInstanceForNavigation(
content::RenderFrameHost* current_rfh,
Expand Down Expand Up @@ -509,6 +517,10 @@ void AtomBrowserClient::AppendExtraCommandLineSwitches(
web_preferences->AppendCommandLineSwitches(command_line);
SessionPreferences::AppendExtraCommandLineSwitches(
web_contents->GetBrowserContext(), command_line);
if (CanUseCustomSiteInstance()) {
command_line->AppendSwitch(
switches::kDisableElectronSiteInstanceOverrides);
}
}
}

Expand Down
5 changes: 5 additions & 0 deletions atom/browser/atom_browser_client.h
Expand Up @@ -67,6 +67,9 @@ class AtomBrowserClient : public content::ContentBrowserClient,
std::string GetUserAgent() const override;
void SetUserAgent(const std::string& user_agent);

void SetCanUseCustomSiteInstance(bool should_disable);
bool CanUseCustomSiteInstance() override;

protected:
void RenderProcessWillLaunch(
content::RenderProcessHost* host,
Expand Down Expand Up @@ -253,6 +256,8 @@ class AtomBrowserClient : public content::ContentBrowserClient,

std::string user_agent_override_ = "";

bool disable_process_restart_tricks_ = false;

DISALLOW_COPY_AND_ASSIGN(AtomBrowserClient);
};

Expand Down
2 changes: 2 additions & 0 deletions atom/common/options_switches.cc
Expand Up @@ -232,6 +232,8 @@ const char kScrollBounce[] = "scroll-bounce";
const char kHiddenPage[] = "hidden-page";
const char kNativeWindowOpen[] = "native-window-open";
const char kWebviewTag[] = "webview-tag";
const char kDisableElectronSiteInstanceOverrides[] =
"disable-electron-site-instance-overrides";

// Command switch passed to renderer process to control nodeIntegration.
const char kNodeIntegrationInWorker[] = "node-integration-in-worker";
Expand Down
1 change: 1 addition & 0 deletions atom/common/options_switches.h
Expand Up @@ -118,6 +118,7 @@ extern const char kNodeIntegrationInWorker[];
extern const char kWebviewTag[];
extern const char kNodeIntegrationInSubFrames[];
extern const char kDisableHtmlFullscreenWindowResize[];
extern const char kDisableElectronSiteInstanceOverrides[];

extern const char kWidevineCdmPath[];
extern const char kWidevineCdmVersion[];
Expand Down
15 changes: 12 additions & 3 deletions atom/renderer/atom_renderer_client.cc
Expand Up @@ -104,6 +104,13 @@ void AtomRendererClient::DidCreateScriptContext(

// Setup node environment for each window.
node::Environment* env = node_bindings_->CreateEnvironment(context);
auto* command_line = base::CommandLine::ForCurrentProcess();
// If we have disabled the site instance overrides we should prevent loading
// any non-context aware native module
if (command_line->HasSwitch(switches::kDisableElectronSiteInstanceOverrides))
env->ForceOnlyContextAwareNativeModules();
env->WarnNonContextAwareNativeModules();

environments_.insert(env);

// Add Electron extended APIs.
Expand Down Expand Up @@ -145,9 +152,11 @@ void AtomRendererClient::WillReleaseScriptContext(
// Destroy the node environment. We only do this if node support has been
// enabled for sub-frames to avoid a change-of-behavior / introduce crashes
// for existing users.
// TODO(MarshallOfSOund): Free the environment regardless of this switch
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kNodeIntegrationInSubFrames))
// We also do this if we have disable electron site instance overrides to
// avoid memory leaks
auto* command_line = base::CommandLine::ForCurrentProcess();
if (command_line->HasSwitch(switches::kNodeIntegrationInSubFrames) ||
command_line->HasSwitch(switches::kDisableElectronSiteInstanceOverrides))
node::FreeEnvironment(env);

// ElectronBindings is tracking node environments.
Expand Down
13 changes: 13 additions & 0 deletions docs/api/app.md
Expand Up @@ -1306,3 +1306,16 @@ This is the user agent that will be used when no user agent is set at the
`webContents` or `session` level. It is useful for ensuring that your entire
app has the same user agent. Set to a custom value as early as possible
in your app's initialization to ensure that your overridden value is used.

### `app.allowRendererProcessReuse`

A `Boolean` which when `true` disables the overrides that Electron has in place
to ensure renderer processes are restarted on every navigation. The current
default value for this property is `false`.

The intention is for these overrides to become disabled by default and then at
some point in the future this property will be removed. This property impacts
which native modules you can use in the renderer process. For more information
on the direction Electron is going with renderer process restarts and usage of
native modules in the renderer process please check out this
[Tracking Issue](https://github.com/electron/electron/issues/18397).
10 changes: 5 additions & 5 deletions patches/common/chromium/blink_world_context.patch
Expand Up @@ -5,7 +5,7 @@ Subject: blink_world_context.patch


diff --git a/third_party/blink/public/web/web_local_frame.h b/third_party/blink/public/web/web_local_frame.h
index 8f34337725ac16cea102030f852c5d8ab1d70fcc..7ff160ccf78f3fdca8c1f63f7c3bd53ab7396b91 100644
index 82fb3fdfe6bfa8c8d885ee133270b6f2564325a8..f3bad71eab608d3b9ac0e08446c9e520f47e9b10 100644
--- a/third_party/blink/public/web/web_local_frame.h
+++ b/third_party/blink/public/web/web_local_frame.h
@@ -355,6 +355,9 @@ class WebLocalFrame : public WebFrame {
Expand All @@ -19,10 +19,10 @@ index 8f34337725ac16cea102030f852c5d8ab1d70fcc..7ff160ccf78f3fdca8c1f63f7c3bd53a
// that the script evaluated to with callback. Script execution can be
// suspend.
diff --git a/third_party/blink/renderer/core/frame/web_local_frame_impl.cc b/third_party/blink/renderer/core/frame/web_local_frame_impl.cc
index bbeaed0122ba447891196852d815fac465040046..0f03d4c3a84afb5203da55a2fc2e2b16d3618710 100644
index c8174f4b93b78b9368ef38ecb56e9eccf0b71467..73c2bed83ce240574065e3699617d091a581e9e9 100644
--- a/third_party/blink/renderer/core/frame/web_local_frame_impl.cc
+++ b/third_party/blink/renderer/core/frame/web_local_frame_impl.cc
@@ -869,6 +869,13 @@ v8::Local<v8::Object> WebLocalFrameImpl::GlobalProxy() const {
@@ -873,6 +873,13 @@ v8::Local<v8::Object> WebLocalFrameImpl::GlobalProxy() const {
return MainWorldScriptContext()->Global();
}

Expand All @@ -37,10 +37,10 @@ index bbeaed0122ba447891196852d815fac465040046..0f03d4c3a84afb5203da55a2fc2e2b16
return BindingSecurity::ShouldAllowAccessToFrame(
CurrentDOMWindow(V8PerIsolateData::MainThreadIsolate()),
diff --git a/third_party/blink/renderer/core/frame/web_local_frame_impl.h b/third_party/blink/renderer/core/frame/web_local_frame_impl.h
index f7884356209f7c1be523de11ce5f97187c6e24d1..fae16b85298bb1086c3ea493ae754683528594a6 100644
index 3db1bec7516a40eb2f654574baa108e99ff9fb2d..fb5b43e48e455d64ce986cb5490291fc1c18d8ba 100644
--- a/third_party/blink/renderer/core/frame/web_local_frame_impl.h
+++ b/third_party/blink/renderer/core/frame/web_local_frame_impl.h
@@ -147,6 +147,8 @@ class CORE_EXPORT WebLocalFrameImpl final
@@ -146,6 +146,8 @@ class CORE_EXPORT WebLocalFrameImpl final
int argc,
v8::Local<v8::Value> argv[]) override;
v8::Local<v8::Context> MainWorldScriptContext() const override;
Expand Down
Expand Up @@ -8,7 +8,7 @@ categories in use are known / declared. This patch is required for us
to introduce a new Electron category.

diff --git a/base/trace_event/builtin_categories.h b/base/trace_event/builtin_categories.h
index 860caacd290b4d0df909a4eacc7a358701d7362a..d79d3f214cedd8df3b5c61d7f91c096701d33bf0 100644
index 97055b4bdaf0dc38b8d248fefddfcf8c68c123b9..6dbe407151dfc5a9168df77f42932e51b802377f 100644
--- a/base/trace_event/builtin_categories.h
+++ b/base/trace_event/builtin_categories.h
@@ -61,6 +61,7 @@
Expand Down
22 changes: 11 additions & 11 deletions patches/common/chromium/can_create_window.patch
Expand Up @@ -5,10 +5,10 @@ Subject: can_create_window.patch


diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index de3b289f7286b5b3fc4550e35a2534217e34e1e2..4e8ce36e862aefa2107d9bc86f92b14e1ec1c323 100644
index 797264d0cb4236ffc91f366cf6be30643cb7bf2b..c0cd0b41088773eb6226f2074d62a798346d1f72 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -3674,6 +3674,7 @@ void RenderFrameHostImpl::CreateNewWindow(
@@ -3712,6 +3712,7 @@ void RenderFrameHostImpl::CreateNewWindow(
last_committed_origin_, params->window_container_type,
params->target_url, params->referrer.To<Referrer>(),
params->frame_name, params->disposition, *params->features,
Expand All @@ -17,7 +17,7 @@ index de3b289f7286b5b3fc4550e35a2534217e34e1e2..4e8ce36e862aefa2107d9bc86f92b14e
&no_javascript_access);

diff --git a/content/common/frame.mojom b/content/common/frame.mojom
index ee0ca5d5b76756ab4123652b02298eede20f22e0..51c89138745cf587a483771b4716b5cabef91eb3 100644
index 82882159b0bac6d47d678c485de0aacc7db06c2d..dd2299094b79d82da7ec1cd8f559050b6f0e2af5 100644
--- a/content/common/frame.mojom
+++ b/content/common/frame.mojom
@@ -291,6 +291,10 @@ struct CreateNewWindowParams {
Expand All @@ -32,7 +32,7 @@ index ee0ca5d5b76756ab4123652b02298eede20f22e0..51c89138745cf587a483771b4716b5ca

// Operation result when the renderer asks the browser to create a new window.
diff --git a/content/public/browser/content_browser_client.cc b/content/public/browser/content_browser_client.cc
index 23b7c63cd45e05830134072fbe7baff86b8380f6..22815b555961fb5aa267f747372b3c3397ea2ae8 100644
index ce9d0ede84da62061278f7fb0c543fc2e8a0216e..3729dcc9ea3272c943754a92c6ed1d7a1fd8fcf3 100644
--- a/content/public/browser/content_browser_client.cc
+++ b/content/public/browser/content_browser_client.cc
@@ -518,6 +518,8 @@ bool ContentBrowserClient::CanCreateWindow(
Expand All @@ -45,7 +45,7 @@ index 23b7c63cd45e05830134072fbe7baff86b8380f6..22815b555961fb5aa267f747372b3c33
bool opener_suppressed,
bool* no_javascript_access) {
diff --git a/content/public/browser/content_browser_client.h b/content/public/browser/content_browser_client.h
index 1041df57b14a4cdca9a542108825ab933311b9ee..89bcd0a8e3ea9b966f6b7c18f0514da83a6b2b98 100644
index 449941ddc4d43dc4cb164f6af9dcc69991dddc8b..4c84fb3648b3de36015b325246559f8aefe2ebd5 100644
--- a/content/public/browser/content_browser_client.h
+++ b/content/public/browser/content_browser_client.h
@@ -177,6 +177,7 @@ class RenderFrameHost;
Expand All @@ -66,7 +66,7 @@ index 1041df57b14a4cdca9a542108825ab933311b9ee..89bcd0a8e3ea9b966f6b7c18f0514da8
bool opener_suppressed,
bool* no_javascript_access);
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
index a9508139d626447ebf54edf1281337b9f7a3f406..ee84732bc535538f95fc2c767060dca50fbdf561 100644
index 49e1e5647f700350a07ad88306a06122d0f0f204..39c5ce2a631cc1d78e36dbda506212b87f5a1939 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -77,6 +77,7 @@
Expand All @@ -77,7 +77,7 @@ index a9508139d626447ebf54edf1281337b9f7a3f406..ee84732bc535538f95fc2c767060dca5
#include "content/renderer/media/audio/audio_device_factory.h"
#include "content/renderer/media/stream/media_stream_device_observer.h"
#include "content/renderer/media/video_capture/video_capture_impl_manager.h"
@@ -1368,6 +1369,8 @@ WebView* RenderViewImpl::CreateView(
@@ -1358,6 +1359,8 @@ WebView* RenderViewImpl::CreateView(
}
params->features = ConvertWebWindowFeaturesToMojoWindowFeatures(features);

Expand All @@ -87,10 +87,10 @@ index a9508139d626447ebf54edf1281337b9f7a3f406..ee84732bc535538f95fc2c767060dca5
// moved on send.
bool is_background_tab =
diff --git a/content/shell/browser/web_test/web_test_content_browser_client.cc b/content/shell/browser/web_test/web_test_content_browser_client.cc
index 04149aa4478cfbd0e1aafaed23dbddca3d3b161a..40d1f8c342c86510a91a973c4aeb1bda5b8e42f9 100644
index 3e3f251af3c531fca379f7fa00f67d671bbe2d5f..e77427146729664247e4dd3313f8533a78059bf7 100644
--- a/content/shell/browser/web_test/web_test_content_browser_client.cc
+++ b/content/shell/browser/web_test/web_test_content_browser_client.cc
@@ -296,6 +296,8 @@ bool WebTestContentBrowserClient::CanCreateWindow(
@@ -299,6 +299,8 @@ bool WebTestContentBrowserClient::CanCreateWindow(
const std::string& frame_name,
WindowOpenDisposition disposition,
const blink::mojom::WindowFeatures& features,
Expand All @@ -100,10 +100,10 @@ index 04149aa4478cfbd0e1aafaed23dbddca3d3b161a..40d1f8c342c86510a91a973c4aeb1bda
bool opener_suppressed,
bool* no_javascript_access) {
diff --git a/content/shell/browser/web_test/web_test_content_browser_client.h b/content/shell/browser/web_test/web_test_content_browser_client.h
index 6413e5f117d7dfd4a61779d4c971c28a14a86082..a02e232249cf99c55ec5b07a94b29f0a6d4a38bf 100644
index 8b9ae118bca4678c315d820af6b4dbe850943ed4..acde862d6d48429db5936f2e6735017dc2ef647e 100644
--- a/content/shell/browser/web_test/web_test_content_browser_client.h
+++ b/content/shell/browser/web_test/web_test_content_browser_client.h
@@ -68,6 +68,8 @@ class WebTestContentBrowserClient : public ShellContentBrowserClient {
@@ -69,6 +69,8 @@ class WebTestContentBrowserClient : public ShellContentBrowserClient {
const std::string& frame_name,
WindowOpenDisposition disposition,
const blink::mojom::WindowFeatures& features,
Expand Down
Expand Up @@ -8,10 +8,10 @@ this patch can be removed once we switch to network service,
where the embedders have a chance to design their URLLoaders.

diff --git a/content/browser/loader/cross_site_document_resource_handler.cc b/content/browser/loader/cross_site_document_resource_handler.cc
index 9e65e50de1d45d8435145b56bf7108a8c0272065..3103e4caa2adf853277774092cbd645fd8ece952 100644
index 2151c5b9698e9a2768875d04a2297956cc4c0d41..8a316a117ab367bcbac947894cbe1bc2428de93e 100644
--- a/content/browser/loader/cross_site_document_resource_handler.cc
+++ b/content/browser/loader/cross_site_document_resource_handler.cc
@@ -666,6 +666,9 @@ bool CrossSiteDocumentResourceHandler::ShouldBlockBasedOnHeaders(
@@ -671,6 +671,9 @@ bool CrossSiteDocumentResourceHandler::ShouldBlockBasedOnHeaders(
return false;
}

Expand All @@ -22,7 +22,7 @@ index 9e65e50de1d45d8435145b56bf7108a8c0272065..3103e4caa2adf853277774092cbd645f
}

diff --git a/content/public/browser/content_browser_client.cc b/content/public/browser/content_browser_client.cc
index 22815b555961fb5aa267f747372b3c3397ea2ae8..dd582ad7e2911d049575a25eb7990e50bdebc73e 100644
index 3729dcc9ea3272c943754a92c6ed1d7a1fd8fcf3..787cd81b26508d3e04956721f0e7cec2f457aa60 100644
--- a/content/public/browser/content_browser_client.cc
+++ b/content/public/browser/content_browser_client.cc
@@ -56,6 +56,10 @@ BrowserMainParts* ContentBrowserClient::CreateBrowserMainParts(
Expand All @@ -37,10 +37,10 @@ index 22815b555961fb5aa267f747372b3c3397ea2ae8..dd582ad7e2911d049575a25eb7990e50
const base::Location& from_here,
const scoped_refptr<base::TaskRunner>& task_runner,
diff --git a/content/public/browser/content_browser_client.h b/content/public/browser/content_browser_client.h
index 89bcd0a8e3ea9b966f6b7c18f0514da83a6b2b98..8e217f7098c766ec708be6f14b5b68456c0cb6f1 100644
index 4c84fb3648b3de36015b325246559f8aefe2ebd5..bf9b3a601fc16d5070e4467a258a047f47b059f3 100644
--- a/content/public/browser/content_browser_client.h
+++ b/content/public/browser/content_browser_client.h
@@ -218,6 +218,9 @@ class CONTENT_EXPORT ContentBrowserClient {
@@ -219,6 +219,9 @@ class CONTENT_EXPORT ContentBrowserClient {
virtual BrowserMainParts* CreateBrowserMainParts(
const MainFunctionParams& parameters);

Expand Down

0 comments on commit 90af9bc

Please sign in to comment.