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

@@ -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);
+  auto* web_contents = content::WebContents::FromRenderViewHost(host);
-  WebContentsPreferences::OverrideWebkitPrefs(web_contents, prefs);
+  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(
+    auto* web_preferences = WebContentsPreferences::From(web_contents);
-        web_contents, command_line);
+    if (web_preferences)
+      web_preferences->AppendCommandLineSwitches(command_line);
     SessionPreferences::AppendExtraCommandLineSwitches(
         web_contents->GetBrowserContext(), command_line);
 

atom/browser/web_contents_preferences.cc

@@ -125,70 +125,55 @@ WebContentsPreferences* WebContentsPreferences::From(int process_id) {
   return From(GetWebContentsFromProcessID(process_id));
 }
 
-// static
+void WebContentsPreferences::AppendCommandLineSwitches(
-void WebContentsPreferences::AppendExtraCommandLineSwitches(
+    base::CommandLine* command_line) {
-    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);
-
   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 (dict_.GetBoolean("sandbox", &b) && b)
-  if (web_preferences.GetBoolean("sandbox", &sandbox) && sandbox) {
     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 (dict_.GetBoolean("nativeWindowOpen", &b) && b)
-  if (web_preferences.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))
+  if (dict_.Get(options::kCustomArgs, &customArgs) &&
-      && (customArgs->is_list())) {
+      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 (dict_.GetBoolean(options::kContextIsolation, &b) && b)
-  if (web_preferences.GetBoolean(options::kContextIsolation, &isolated) &&
-      isolated)
     command_line->AppendSwitch(switches::kContextIsolation);
 
   // --background-color.
-  std::string color;
+  std::string s;
-  if (web_preferences.GetString(options::kBackgroundColor, &color))
+  if (dict_.GetString(options::kBackgroundColor, &s))
-    command_line->AppendSwitchASCII(switches::kBackgroundColor, color);
+    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 (dict_.GetBoolean(options::kScrollBounce, &b) && b)
-  if (web_preferences.GetBoolean(options::kScrollBounce, &scroll_bounce) &&
-      scroll_bounce)
     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 (dict_.GetString(options::kBlinkFeatures, &s))
-  if (web_preferences.GetString(options::kBlinkFeatures, &blink_features))
+    command_line->AppendSwitchASCII(::switches::kEnableBlinkFeatures, s);
-    command_line->AppendSwitchASCII(::switches::kEnableBlinkFeatures,
-                                    blink_features);
 
   // Disable blink features.
-  std::string disable_blink_features;
+  if (dict_.GetString(options::kDisableBlinkFeatures, &s))
-  if (web_preferences.GetString(options::kDisableBlinkFeatures,
+    command_line->AppendSwitchASCII(::switches::kDisableBlinkFeatures, s);
-                                &disable_blink_features))
-    command_line->AppendSwitchASCII(::switches::kDisableBlinkFeatures,
-                                    disable_blink_features);
 
   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) {
+    content::WebPreferences* prefs) {
-  WebContentsPreferences* self = FromWebContents(web_contents);
-  if (!self)
-    return;
-
   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;
 }
 

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_; }