From c5ad7ed0cd05fe46aefedd0c3dbed580d6a5ce20 Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Thu, 29 Jul 2021 00:32:53 +0200 Subject: [PATCH] refactor: remove guestInstanceId from WebPreferences (#30280) * refactor: remove guestInstanceId from WebPreferences * refactor: remove WebViewManager::GetEmbedder --- lib/browser/guest-view-manager.ts | 5 +- lib/renderer/init.ts | 6 +-- lib/renderer/web-view/web-view-init.ts | 6 +-- lib/renderer/window-setup.ts | 6 +-- lib/sandboxed_renderer/init.ts | 6 +-- ..._windows_to_have_different_web_prefs.patch | 22 ++++---- shell/browser/web_contents_preferences.cc | 52 +++++++++---------- shell/browser/web_contents_preferences.h | 2 +- shell/browser/web_view_manager.cc | 6 --- shell/browser/web_view_manager.h | 1 - shell/common/options_switches.cc | 3 -- shell/common/options_switches.h | 1 - shell/renderer/api/electron_api_web_frame.cc | 5 +- typings/internal-ambient.d.ts | 2 +- typings/internal-electron.d.ts | 1 - 15 files changed, 53 insertions(+), 71 deletions(-) diff --git a/lib/browser/guest-view-manager.ts b/lib/browser/guest-view-manager.ts index 41863541d0..236b821c6d 100644 --- a/lib/browser/guest-view-manager.ts +++ b/lib/browser/guest-view-manager.ts @@ -28,7 +28,7 @@ function sanitizeOptionsForGuest (options: Record) { return ret; } -function makeWebPreferences (embedder: Electron.WebContents, params: Record, guestInstanceId: number) { +function makeWebPreferences (embedder: Electron.WebContents, params: Record) { // parse the 'webpreferences' attribute string, if set // this uses the same parsing rules as window.open uses for its features const parsedWebPreferences = @@ -37,7 +37,6 @@ function makeWebPreferences (embedder: Electron.WebContents, params: Record`. - if (webviewTag && !guestInstanceId) { + if (webviewTag && !isWebView) { const guestViewInternal = require('@electron/internal/renderer/web-view/guest-view-internal') as typeof guestViewInternalModule; if (contextIsolation) { v8Util.setHiddenValue(window, 'guestViewInternal', guestViewInternal); @@ -36,7 +36,7 @@ export function webViewInit (contextIsolation: boolean, webviewTag: boolean, gue } } - if (guestInstanceId) { + if (isWebView) { // Report focus/blur events of webview to browser. handleFocusBlur(); } diff --git a/lib/renderer/window-setup.ts b/lib/renderer/window-setup.ts index 1b12c3bbc8..de7bf37da5 100644 --- a/lib/renderer/window-setup.ts +++ b/lib/renderer/window-setup.ts @@ -243,8 +243,8 @@ class BrowserWindowProxy { } export const windowSetup = ( - guestInstanceId: number, openerId: number, isHiddenPage: boolean, usesNativeWindowOpen: boolean) => { - if (!process.sandboxed && !guestInstanceId) { + isWebView: boolean, openerId: number, isHiddenPage: boolean, usesNativeWindowOpen: boolean) => { + if (!process.sandboxed && !isWebView) { // Override default window.close. window.close = function () { ipcRendererInternal.send(IPC_MESSAGES.BROWSER_WINDOW_CLOSE); @@ -318,7 +318,7 @@ export const windowSetup = ( }); } - if (guestInstanceId) { + if (isWebView) { // Webview `document.visibilityState` tracks window visibility (and ignores // the actual element visibility) for backwards compatibility. // See discussion in #9178. diff --git a/lib/sandboxed_renderer/init.ts b/lib/sandboxed_renderer/init.ts index cd76c379af..a91a6b6692 100644 --- a/lib/sandboxed_renderer/init.ts +++ b/lib/sandboxed_renderer/init.ts @@ -127,7 +127,7 @@ const contextIsolation = mainFrame.getWebPreference('contextIsolation'); const webviewTag = mainFrame.getWebPreference('webviewTag'); const isHiddenPage = mainFrame.getWebPreference('hiddenPage'); const usesNativeWindowOpen = true; -const guestInstanceId = mainFrame.getWebPreference('guestInstanceId'); +const isWebView = mainFrame.getWebPreference('isWebView'); const openerId = mainFrame.getWebPreference('openerId'); switch (window.location.protocol) { @@ -145,14 +145,14 @@ switch (window.location.protocol) { default: { // Override default web functions. const { windowSetup } = require('@electron/internal/renderer/window-setup') as typeof windowSetupModule; - windowSetup(guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen); + windowSetup(isWebView, openerId, isHiddenPage, usesNativeWindowOpen); } } // Load webview tag implementation. if (process.isMainFrame) { const { webViewInit } = require('@electron/internal/renderer/web-view/web-view-init') as typeof webViewInitModule; - webViewInit(contextIsolation, webviewTag, guestInstanceId); + webViewInit(contextIsolation, webviewTag, isWebView); } // Wrap the script into a function executed in global scope. It won't have diff --git a/patches/chromium/allow_in-process_windows_to_have_different_web_prefs.patch b/patches/chromium/allow_in-process_windows_to_have_different_web_prefs.patch index 6df6c791c7..7b0934d582 100644 --- a/patches/chromium/allow_in-process_windows_to_have_different_web_prefs.patch +++ b/patches/chromium/allow_in-process_windows_to_have_different_web_prefs.patch @@ -8,7 +8,7 @@ WebPreferences of in-process child windows, rather than relying on process-level command line switches, as before. diff --git a/third_party/blink/common/web_preferences/web_preferences.cc b/third_party/blink/common/web_preferences/web_preferences.cc -index 8a1315f7f89588bb21c6d3c21a7de7c07fed9679..51cc790f7e1ba1440a6ffaa56c9e01687fc35c06 100644 +index 8a1315f7f89588bb21c6d3c21a7de7c07fed9679..c23443c7816e38836b2f4233ca3a742693888f39 100644 --- a/third_party/blink/common/web_preferences/web_preferences.cc +++ b/third_party/blink/common/web_preferences/web_preferences.cc @@ -148,6 +148,22 @@ WebPreferences::WebPreferences() @@ -18,7 +18,7 @@ index 8a1315f7f89588bb21c6d3c21a7de7c07fed9679..51cc790f7e1ba1440a6ffaa56c9e0168 + // Begin Electron-specific WebPreferences. + opener_id(0), + context_isolation(false), -+ guest_instance_id(0), ++ is_webview(false), + hidden_page(false), + offscreen(false), + preload(base::FilePath::StringType()), @@ -35,7 +35,7 @@ index 8a1315f7f89588bb21c6d3c21a7de7c07fed9679..51cc790f7e1ba1440a6ffaa56c9e0168 accelerated_video_decode_enabled(false), animation_policy( diff --git a/third_party/blink/common/web_preferences/web_preferences_mojom_traits.cc b/third_party/blink/common/web_preferences/web_preferences_mojom_traits.cc -index a264ef99beb81dd6b1f55c1b0f57f6055b4ab771..ff4d5c5245ba0641b4ef02cdaf5d50be537b709e 100644 +index a264ef99beb81dd6b1f55c1b0f57f6055b4ab771..16b734cb371d22dbe99b4ae397126f240b9f686c 100644 --- a/third_party/blink/common/web_preferences/web_preferences_mojom_traits.cc +++ b/third_party/blink/common/web_preferences/web_preferences_mojom_traits.cc @@ -23,6 +23,10 @@ bool StructTraitsopener_id = data.opener_id(); + out->context_isolation = data.context_isolation(); -+ out->guest_instance_id = data.guest_instance_id(); ++ out->is_webview = data.is_webview(); + out->hidden_page = data.hidden_page(); + out->offscreen = data.offscreen(); + out->native_window_open = data.native_window_open(); @@ -72,7 +72,7 @@ index a264ef99beb81dd6b1f55c1b0f57f6055b4ab771..ff4d5c5245ba0641b4ef02cdaf5d50be out->accelerated_video_decode_enabled = data.accelerated_video_decode_enabled(); diff --git a/third_party/blink/public/common/web_preferences/web_preferences.h b/third_party/blink/public/common/web_preferences/web_preferences.h -index 4517bf43c1b80f1aa0f3ba8e67e78b8b91e19f8a..492f6c948af74bb925826a1075e6343f147f0eb2 100644 +index 4517bf43c1b80f1aa0f3ba8e67e78b8b91e19f8a..5dc73459f424ad0a01b4ea18e4de196e20d24ae5 100644 --- a/third_party/blink/public/common/web_preferences/web_preferences.h +++ b/third_party/blink/public/common/web_preferences/web_preferences.h @@ -10,6 +10,7 @@ @@ -91,7 +91,7 @@ index 4517bf43c1b80f1aa0f3ba8e67e78b8b91e19f8a..492f6c948af74bb925826a1075e6343f + std::vector preloads; + int opener_id; + bool context_isolation; -+ int guest_instance_id; ++ bool is_webview; + bool hidden_page; + bool offscreen; + base::FilePath preload; @@ -109,7 +109,7 @@ index 4517bf43c1b80f1aa0f3ba8e67e78b8b91e19f8a..492f6c948af74bb925826a1075e6343f // only controls whether or not the "document.cookie" field is properly // connected to the backing store, for instance if you wanted to be able to diff --git a/third_party/blink/public/common/web_preferences/web_preferences_mojom_traits.h b/third_party/blink/public/common/web_preferences/web_preferences_mojom_traits.h -index 9dbbb581a8876430c3e0a39df1ff655d3ddc6d2d..fd6cbcfa1992a75bf660fb9eb47a9099acb3834f 100644 +index 9dbbb581a8876430c3e0a39df1ff655d3ddc6d2d..98c5c4ac5ee3130581c8576e5ef810a78939f50e 100644 --- a/third_party/blink/public/common/web_preferences/web_preferences_mojom_traits.h +++ b/third_party/blink/public/common/web_preferences/web_preferences_mojom_traits.h @@ -6,6 +6,7 @@ @@ -137,8 +137,8 @@ index 9dbbb581a8876430c3e0a39df1ff655d3ddc6d2d..fd6cbcfa1992a75bf660fb9eb47a9099 + return r.context_isolation; + } + -+ static int guest_instance_id(const blink::web_pref::WebPreferences& r) { -+ return r.guest_instance_id; ++ static int is_webview(const blink::web_pref::WebPreferences& r) { ++ return r.is_webview; + } + + static bool hidden_page(const blink::web_pref::WebPreferences& r) { @@ -190,7 +190,7 @@ index 9dbbb581a8876430c3e0a39df1ff655d3ddc6d2d..fd6cbcfa1992a75bf660fb9eb47a9099 return r.cookie_enabled; } diff --git a/third_party/blink/public/mojom/webpreferences/web_preferences.mojom b/third_party/blink/public/mojom/webpreferences/web_preferences.mojom -index eaecb8c2b7dadaf7650bc8ac85cbad4383035e67..7863c865df17fa95965b5a4e543579bac22af90b 100644 +index eaecb8c2b7dadaf7650bc8ac85cbad4383035e67..64d83af36d384d2eb7fdd89a3a0d3665a40354a1 100644 --- a/third_party/blink/public/mojom/webpreferences/web_preferences.mojom +++ b/third_party/blink/public/mojom/webpreferences/web_preferences.mojom @@ -10,6 +10,7 @@ import "third_party/blink/public/mojom/v8_cache_options.mojom"; @@ -209,7 +209,7 @@ index eaecb8c2b7dadaf7650bc8ac85cbad4383035e67..7863c865df17fa95965b5a4e543579ba + array preloads; + int32 opener_id; + bool context_isolation; -+ int32 guest_instance_id; ++ bool is_webview; + bool hidden_page; + bool offscreen; + mojo_base.mojom.FilePath preload; diff --git a/shell/browser/web_contents_preferences.cc b/shell/browser/web_contents_preferences.cc index 2d1491438b..cdc31036bc 100644 --- a/shell/browser/web_contents_preferences.cc +++ b/shell/browser/web_contents_preferences.cc @@ -19,8 +19,8 @@ #include "content/public/common/content_switches.h" #include "net/base/filename_util.h" #include "sandbox/policy/switches.h" +#include "shell/browser/api/electron_api_web_contents.h" #include "shell/browser/native_window.h" -#include "shell/browser/web_view_manager.h" #include "shell/common/gin_converters/value_converter.h" #include "shell/common/gin_helper/dictionary.h" #include "shell/common/options_switches.h" @@ -103,15 +103,12 @@ WebContentsPreferences::WebContentsPreferences( // If this is a tag, and the embedder is offscreen-rendered, then // this WebContents is also offscreen-rendered. - if (guest_instance_id_) { - auto* manager = WebViewManager::GetWebViewManager(web_contents); - if (manager) { - auto* embedder = manager->GetEmbedder(guest_instance_id_); - if (embedder) { - auto* embedder_preferences = WebContentsPreferences::From(embedder); - if (embedder_preferences && embedder_preferences->IsOffscreen()) { - offscreen_ = true; - } + if (auto* api_web_contents = api::WebContents::From(web_contents_)) { + if (electron::api::WebContents* embedder = api_web_contents->embedder()) { + auto* embedder_preferences = + WebContentsPreferences::From(embedder->web_contents()); + if (embedder_preferences && embedder_preferences->IsOffscreen()) { + offscreen_ = true; } } } @@ -150,7 +147,7 @@ void WebContentsPreferences::Clear() { minimum_font_size_ = absl::nullopt; default_encoding_ = absl::nullopt; opener_id_ = 0; - guest_instance_id_ = 0; + is_webview_ = false; custom_args_.clear(); custom_switches_.clear(); enable_blink_features_ = absl::nullopt; @@ -223,7 +220,6 @@ void WebContentsPreferences::Merge( if (web_preferences.Get("defaultEncoding", &encoding)) default_encoding_ = encoding; web_preferences.Get(options::kOpenerID, &opener_id_); - web_preferences.Get(options::kGuestInstanceID, &guest_instance_id_); web_preferences.Get(options::kCustomArgs, &custom_args_); web_preferences.Get("commandLineSwitches", &custom_switches_); web_preferences.Get("disablePopups", &disable_popups_); @@ -263,6 +259,11 @@ void WebContentsPreferences::Merge( } } + std::string type; + if (web_preferences.Get(options::kType, &type)) { + is_webview_ = type == "webview"; + } + web_preferences.Get("v8CacheOptions", &v8_cache_options_); #if defined(OS_MAC) @@ -459,24 +460,19 @@ void WebContentsPreferences::OverrideWebkitPrefs( // Run Electron APIs and preload script in isolated world prefs->context_isolation = context_isolation_; - prefs->guest_instance_id = guest_instance_id_; + prefs->is_webview = is_webview_; prefs->hidden_page = false; - if (guest_instance_id_) { - // Webview `document.visibilityState` tracks window visibility so we need - // to let it know if the window happens to be hidden right now. - auto* manager = WebViewManager::GetWebViewManager(web_contents_); - if (manager) { - auto* embedder = manager->GetEmbedder(guest_instance_id_); - if (embedder) { - auto* relay = NativeWindowRelay::FromWebContents(embedder); - if (relay) { - auto* window = relay->GetNativeWindow(); - if (window) { - const bool visible = window->IsVisible() && !window->IsMinimized(); - if (!visible) { - prefs->hidden_page = true; - } + // Webview `document.visibilityState` tracks window visibility so we need + // to let it know if the window happens to be hidden right now. + if (auto* api_web_contents = api::WebContents::From(web_contents_)) { + if (electron::api::WebContents* embedder = api_web_contents->embedder()) { + if (auto* relay = + NativeWindowRelay::FromWebContents(embedder->web_contents())) { + if (auto* window = relay->GetNativeWindow()) { + const bool visible = window->IsVisible() && !window->IsMinimized(); + if (!visible) { + prefs->hidden_page = true; } } } diff --git a/shell/browser/web_contents_preferences.h b/shell/browser/web_contents_preferences.h index 4db482477c..4cc7e27d44 100644 --- a/shell/browser/web_contents_preferences.h +++ b/shell/browser/web_contents_preferences.h @@ -110,7 +110,7 @@ class WebContentsPreferences absl::optional minimum_font_size_; absl::optional default_encoding_; int opener_id_; - int guest_instance_id_; + bool is_webview_; std::vector custom_args_; std::vector custom_switches_; absl::optional enable_blink_features_; diff --git a/shell/browser/web_view_manager.cc b/shell/browser/web_view_manager.cc index 54a424144e..f24a5d1b83 100644 --- a/shell/browser/web_view_manager.cc +++ b/shell/browser/web_view_manager.cc @@ -23,12 +23,6 @@ void WebViewManager::RemoveGuest(int guest_instance_id) { web_contents_embedder_map_.erase(guest_instance_id); } -content::WebContents* WebViewManager::GetEmbedder(int guest_instance_id) { - const auto iter = web_contents_embedder_map_.find(guest_instance_id); - return iter == std::end(web_contents_embedder_map_) ? nullptr - : iter->second.embedder; -} - bool WebViewManager::ForEachGuest(content::WebContents* embedder_web_contents, const GuestCallback& callback) { for (auto& item : web_contents_embedder_map_) { diff --git a/shell/browser/web_view_manager.h b/shell/browser/web_view_manager.h index 695a29ed40..5f8b247b40 100644 --- a/shell/browser/web_view_manager.h +++ b/shell/browser/web_view_manager.h @@ -20,7 +20,6 @@ class WebViewManager : public content::BrowserPluginGuestManager { content::WebContents* embedder, content::WebContents* web_contents); void RemoveGuest(int guest_instance_id); - content::WebContents* GetEmbedder(int guest_instance_id); static WebViewManager* GetWebViewManager(content::WebContents* web_contents); diff --git a/shell/common/options_switches.cc b/shell/common/options_switches.cc index da619b9f5f..761ff2982e 100644 --- a/shell/common/options_switches.cc +++ b/shell/common/options_switches.cc @@ -121,9 +121,6 @@ const char kNodeIntegration[] = "nodeIntegration"; // Enable context isolation of Electron APIs and preload script const char kContextIsolation[] = "contextIsolation"; -// Instance ID of guest WebContents. -const char kGuestInstanceID[] = "guestInstanceId"; - // Web runtime features. const char kExperimentalFeatures[] = "experimentalFeatures"; diff --git a/shell/common/options_switches.h b/shell/common/options_switches.h index 47d0db155b..ce44b3440c 100644 --- a/shell/common/options_switches.h +++ b/shell/common/options_switches.h @@ -66,7 +66,6 @@ extern const char kPreloadScripts[]; extern const char kPreloadURL[]; extern const char kNodeIntegration[]; extern const char kContextIsolation[]; -extern const char kGuestInstanceID[]; extern const char kExperimentalFeatures[]; extern const char kOpenerID[]; extern const char kScrollBounce[]; diff --git a/shell/renderer/api/electron_api_web_frame.cc b/shell/renderer/api/electron_api_web_frame.cc index ad786c0f5d..fdfef420f2 100644 --- a/shell/renderer/api/electron_api_web_frame.cc +++ b/shell/renderer/api/electron_api_web_frame.cc @@ -493,9 +493,8 @@ class WebFrameRenderer : public gin::Wrappable, return gin::ConvertToV8(isolate, prefs.opener_id); } else if (pref_name == options::kContextIsolation) { return gin::ConvertToV8(isolate, prefs.context_isolation); - } else if (pref_name == options::kGuestInstanceID) { - // NOTE: guestInstanceId is internal-only. - return gin::ConvertToV8(isolate, prefs.guest_instance_id); + } else if (pref_name == "isWebView") { + return gin::ConvertToV8(isolate, prefs.is_webview); } else if (pref_name == options::kHiddenPage) { // NOTE: hiddenPage is internal-only. return gin::ConvertToV8(isolate, prefs.hidden_page); diff --git a/typings/internal-ambient.d.ts b/typings/internal-ambient.d.ts index 590e8d7cab..f5a803b79d 100644 --- a/typings/internal-ambient.d.ts +++ b/typings/internal-ambient.d.ts @@ -108,7 +108,7 @@ declare namespace NodeJS { interface InternalWebPreferences { contextIsolation: boolean; - guestInstanceId: number; + isWebView: boolean; hiddenPage: boolean; nativeWindowOpen: boolean; nodeIntegration: boolean; diff --git a/typings/internal-electron.d.ts b/typings/internal-electron.d.ts index efd12313e8..ac76cf2032 100644 --- a/typings/internal-electron.d.ts +++ b/typings/internal-electron.d.ts @@ -90,7 +90,6 @@ declare namespace Electron { } interface WebPreferences { - guestInstanceId?: number; openerId?: number | null; disablePopups?: boolean; preloadURL?: string;