From 6df2326a3084dd4603afc6c7cdf2c18f6b131533 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Thu, 8 Mar 2018 17:01:54 +0900 Subject: [PATCH] Cleanup the static methods of WebContentsPreferences The static methods are totally unnecessary, and it makes code harder to understand since we are using different ways to do the same things. --- atom/browser/atom_browser_client.cc | 11 +- atom/browser/web_contents_preferences.cc | 122 +++++++++-------------- atom/browser/web_contents_preferences.h | 14 ++- 3 files changed, 62 insertions(+), 85 deletions(-) diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index 5a6aa19a56e7..cf7cc2c96939 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -208,8 +208,10 @@ void AtomBrowserClient::OverrideWebkitPrefs( prefs->allow_running_insecure_content = false; // Custom preferences of guest page. - auto web_contents = content::WebContents::FromRenderViewHost(host); - WebContentsPreferences::OverrideWebkitPrefs(web_contents, prefs); + auto* web_contents = content::WebContents::FromRenderViewHost(host); + auto* web_preferences = WebContentsPreferences::From(web_contents); + if (web_preferences) + web_preferences->OverrideWebkitPrefs(prefs); } void AtomBrowserClient::OverrideSiteInstanceForNavigation( @@ -319,8 +321,9 @@ void AtomBrowserClient::AppendExtraCommandLineSwitches( content::WebContents* web_contents = GetWebContentsFromProcessID(process_id); if (web_contents) { - WebContentsPreferences::AppendExtraCommandLineSwitches( - web_contents, command_line); + auto* web_preferences = WebContentsPreferences::From(web_contents); + if (web_preferences) + web_preferences->AppendCommandLineSwitches(command_line); SessionPreferences::AppendExtraCommandLineSwitches( web_contents->GetBrowserContext(), command_line); diff --git a/atom/browser/web_contents_preferences.cc b/atom/browser/web_contents_preferences.cc index 02bea454fb91..8a081b6039b9 100644 --- a/atom/browser/web_contents_preferences.cc +++ b/atom/browser/web_contents_preferences.cc @@ -125,70 +125,55 @@ WebContentsPreferences* WebContentsPreferences::From(int process_id) { return From(GetWebContentsFromProcessID(process_id)); } -// static -void WebContentsPreferences::AppendExtraCommandLineSwitches( - content::WebContents* web_contents, base::CommandLine* command_line) { - WebContentsPreferences* self = FromWebContents(web_contents); - if (!self) - return; - - base::DictionaryValue& web_preferences = self->dict_; - - // We are appending args to a webContents so let's save the current state - // of our preferences object so that during the lifetime of the WebContents - // we can fetch the options used to initally configure the WebContents - self->last_dict_.Clear(); - self->last_dict_.MergeDictionary(&web_preferences); - +void WebContentsPreferences::AppendCommandLineSwitches( + base::CommandLine* command_line) { bool b; // Check if plugins are enabled. - if (web_preferences.GetBoolean("plugins", &b) && b) + if (dict_.GetBoolean("plugins", &b) && b) command_line->AppendSwitch(switches::kEnablePlugins); // Experimental flags. - if (web_preferences.GetBoolean(options::kExperimentalFeatures, &b) && b) + if (dict_.GetBoolean(options::kExperimentalFeatures, &b) && b) command_line->AppendSwitch( ::switches::kEnableExperimentalWebPlatformFeatures); - if (web_preferences.GetBoolean(options::kExperimentalCanvasFeatures, &b) && b) + if (dict_.GetBoolean(options::kExperimentalCanvasFeatures, &b) && b) command_line->AppendSwitch(::switches::kEnableExperimentalCanvasFeatures); // Check if we have node integration specified. bool node_integration = true; - web_preferences.GetBoolean(options::kNodeIntegration, &node_integration); + dict_.GetBoolean(options::kNodeIntegration, &node_integration); command_line->AppendSwitchASCII(switches::kNodeIntegration, node_integration ? "true" : "false"); // Whether to enable node integration in Worker. - if (web_preferences.GetBoolean(options::kNodeIntegrationInWorker, &b) && b) + if (dict_.GetBoolean(options::kNodeIntegrationInWorker, &b) && b) command_line->AppendSwitch(switches::kNodeIntegrationInWorker); // Check if webview tag creation is enabled, default to nodeIntegration value. // TODO(kevinsawicki): Default to false in 2.0 bool webview_tag = node_integration; - web_preferences.GetBoolean(options::kWebviewTag, &webview_tag); + dict_.GetBoolean(options::kWebviewTag, &webview_tag); command_line->AppendSwitchASCII(switches::kWebviewTag, webview_tag ? "true" : "false"); // If the `sandbox` option was passed to the BrowserWindow's webPreferences, // pass `--enable-sandbox` to the renderer so it won't have any node.js // integration. - bool sandbox = false; - if (web_preferences.GetBoolean("sandbox", &sandbox) && sandbox) { + if (dict_.GetBoolean("sandbox", &b) && b) command_line->AppendSwitch(switches::kEnableSandbox); - } else if (!command_line->HasSwitch(switches::kEnableSandbox)) { + else if (!command_line->HasSwitch(switches::kEnableSandbox)) command_line->AppendSwitch(::switches::kNoSandbox); - } - if (web_preferences.GetBoolean("nativeWindowOpen", &b) && b) + if (dict_.GetBoolean("nativeWindowOpen", &b) && b) command_line->AppendSwitch(switches::kNativeWindowOpen); // The preload script. base::FilePath::StringType preload; - if (web_preferences.GetString(options::kPreloadScript, &preload)) { + if (dict_.GetString(options::kPreloadScript, &preload)) { if (base::FilePath(preload).IsAbsolute()) command_line->AppendSwitchNative(switches::kPreloadScript, preload); else LOG(ERROR) << "preload script must have absolute path."; - } else if (web_preferences.GetString(options::kPreloadURL, &preload)) { + } else if (dict_.GetString(options::kPreloadURL, &preload)) { // Translate to file path if there is "preload-url" option. base::FilePath preload_path; if (net::FileURLToFilePath(GURL(preload), &preload_path)) @@ -199,49 +184,44 @@ void WebContentsPreferences::AppendExtraCommandLineSwitches( // Custom args for renderer process base::Value* customArgs; - if ((web_preferences.Get(options::kCustomArgs, &customArgs)) - && (customArgs->is_list())) { + if (dict_.Get(options::kCustomArgs, &customArgs) && + customArgs->is_list()) { for (const base::Value& customArg : customArgs->GetList()) { - if (customArg.is_string()) { + if (customArg.is_string()) command_line->AppendArg(customArg.GetString()); - } } } // Run Electron APIs and preload script in isolated world - bool isolated; - if (web_preferences.GetBoolean(options::kContextIsolation, &isolated) && - isolated) + if (dict_.GetBoolean(options::kContextIsolation, &b) && b) command_line->AppendSwitch(switches::kContextIsolation); // --background-color. - std::string color; - if (web_preferences.GetString(options::kBackgroundColor, &color)) - command_line->AppendSwitchASCII(switches::kBackgroundColor, color); + std::string s; + if (dict_.GetString(options::kBackgroundColor, &s)) + command_line->AppendSwitchASCII(switches::kBackgroundColor, s); // --guest-instance-id, which is used to identify guest WebContents. int guest_instance_id = 0; - if (web_preferences.GetInteger(options::kGuestInstanceID, &guest_instance_id)) + if (dict_.GetInteger(options::kGuestInstanceID, &guest_instance_id)) command_line->AppendSwitchASCII(switches::kGuestInstanceID, base::IntToString(guest_instance_id)); // Pass the opener's window id. int opener_id; - if (web_preferences.GetInteger(options::kOpenerID, &opener_id)) + if (dict_.GetInteger(options::kOpenerID, &opener_id)) command_line->AppendSwitchASCII(switches::kOpenerID, base::IntToString(opener_id)); #if defined(OS_MACOSX) // Enable scroll bounce. - bool scroll_bounce; - if (web_preferences.GetBoolean(options::kScrollBounce, &scroll_bounce) && - scroll_bounce) + if (dict_.GetBoolean(options::kScrollBounce, &b) && b) command_line->AppendSwitch(switches::kScrollBounce); #endif // Custom command line switches. const base::ListValue* args; - if (web_preferences.GetList("commandLineSwitches", &args)) { + if (dict_.GetList("commandLineSwitches", &args)) { for (size_t i = 0; i < args->GetSize(); ++i) { std::string arg; if (args->GetString(i, &arg) && !arg.empty()) @@ -250,26 +230,21 @@ void WebContentsPreferences::AppendExtraCommandLineSwitches( } // Enable blink features. - std::string blink_features; - if (web_preferences.GetString(options::kBlinkFeatures, &blink_features)) - command_line->AppendSwitchASCII(::switches::kEnableBlinkFeatures, - blink_features); + if (dict_.GetString(options::kBlinkFeatures, &s)) + command_line->AppendSwitchASCII(::switches::kEnableBlinkFeatures, s); // Disable blink features. - std::string disable_blink_features; - if (web_preferences.GetString(options::kDisableBlinkFeatures, - &disable_blink_features)) - command_line->AppendSwitchASCII(::switches::kDisableBlinkFeatures, - disable_blink_features); + if (dict_.GetString(options::kDisableBlinkFeatures, &s)) + command_line->AppendSwitchASCII(::switches::kDisableBlinkFeatures, s); 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); + auto* manager = WebViewManager::GetWebViewManager(web_contents_); if (manager) { - auto embedder = manager->GetEmbedder(guest_instance_id); + auto* embedder = manager->GetEmbedder(guest_instance_id); if (embedder) { - auto* relay = NativeWindowRelay::FromWebContents(web_contents); + auto* relay = NativeWindowRelay::FromWebContents(embedder); if (relay) { auto* window = relay->window.get(); if (window) { @@ -282,34 +257,35 @@ void WebContentsPreferences::AppendExtraCommandLineSwitches( } } } + + // We are appending args to a webContents so let's save the current state + // of our preferences object so that during the lifetime of the WebContents + // we can fetch the options used to initally configure the WebContents + last_dict_.Clear(); + last_dict_.MergeDictionary(&dict_); } -// static void WebContentsPreferences::OverrideWebkitPrefs( - content::WebContents* web_contents, content::WebPreferences* prefs) { - WebContentsPreferences* self = FromWebContents(web_contents); - if (!self) - return; - + content::WebPreferences* prefs) { bool b; - if (self->dict_.GetBoolean("javascript", &b)) + if (dict_.GetBoolean("javascript", &b)) prefs->javascript_enabled = b; - if (self->dict_.GetBoolean("images", &b)) + if (dict_.GetBoolean("images", &b)) prefs->images_enabled = b; - if (self->dict_.GetBoolean("textAreasAreResizable", &b)) + if (dict_.GetBoolean("textAreasAreResizable", &b)) prefs->text_areas_are_resizable = b; - if (self->dict_.GetBoolean("webgl", &b)) { + if (dict_.GetBoolean("webgl", &b)) { prefs->webgl1_enabled = b; prefs->webgl2_enabled = b; } - if (self->dict_.GetBoolean("webSecurity", &b)) { + if (dict_.GetBoolean("webSecurity", &b)) { prefs->web_security_enabled = b; prefs->allow_running_insecure_content = !b; } - if (self->dict_.GetBoolean("allowRunningInsecureContent", &b)) + if (dict_.GetBoolean("allowRunningInsecureContent", &b)) prefs->allow_running_insecure_content = b; const base::DictionaryValue* fonts = nullptr; - if (self->dict_.GetDictionary("defaultFontFamily", &fonts)) { + if (dict_.GetDictionary("defaultFontFamily", &fonts)) { base::string16 font; if (fonts->GetString("standard", &font)) prefs->standard_font_family_map[content::kCommonScript] = font; @@ -325,14 +301,14 @@ void WebContentsPreferences::OverrideWebkitPrefs( prefs->fantasy_font_family_map[content::kCommonScript] = font; } int size; - if (self->GetInteger("defaultFontSize", &size)) + if (GetInteger("defaultFontSize", &size)) prefs->default_font_size = size; - if (self->GetInteger("defaultMonospaceFontSize", &size)) + if (GetInteger("defaultMonospaceFontSize", &size)) prefs->default_fixed_font_size = size; - if (self->GetInteger("minimumFontSize", &size)) + if (GetInteger("minimumFontSize", &size)) prefs->minimum_font_size = size; std::string encoding; - if (self->dict_.GetString("defaultEncoding", &encoding)) + if (dict_.GetString("defaultEncoding", &encoding)) prefs->default_encoding = encoding; } diff --git a/atom/browser/web_contents_preferences.h b/atom/browser/web_contents_preferences.h index 1ab2c7ab3e19..c44fa0b46046 100644 --- a/atom/browser/web_contents_preferences.h +++ b/atom/browser/web_contents_preferences.h @@ -34,14 +34,6 @@ class WebContentsPreferences // Get self from procese ID. static WebContentsPreferences* From(int process_id); - // Append command paramters according to |web_contents|'s preferences. - static void AppendExtraCommandLineSwitches( - content::WebContents* web_contents, base::CommandLine* command_line); - - // Modify the WebPreferences according to |web_contents|'s preferences. - static void OverrideWebkitPrefs( - content::WebContents* web_contents, content::WebPreferences* prefs); - WebContentsPreferences(content::WebContents* web_contents, const mate::Dictionary& web_preferences); ~WebContentsPreferences() override; @@ -52,6 +44,12 @@ class WebContentsPreferences // $.extend(|web_preferences|, |new_web_preferences|). void Merge(const base::DictionaryValue& new_web_preferences); + // Append command paramters according to preferences. + void AppendCommandLineSwitches(base::CommandLine* command_line); + + // Modify the WebPreferences according to preferences. + void OverrideWebkitPrefs(content::WebPreferences* prefs); + // Returns the web preferences. base::DictionaryValue* dict() { return &dict_; } const base::DictionaryValue* dict() const { return &dict_; }