Remove kInstantProcess from renderer ContentSettingsObserver

The kInstantProcess flag was used to check if the current frame belongs to instant and should be whitelisted. This would cause layering troubles once ContentSettingsObserver is componentised.

Instead, this CL moves the check back to the construction of the observer, in ChromeContentRendererClient, and only records the Boolean flag for whitelisting in the observer.

BUG=384874

Review URL: https://codereview.chromium.org/797993002

Cr-Commit-Position: refs/heads/master@{#308106}

chrome/renderer/chrome_content_renderer_client.cc

@@ -444,12 +444,14 @@ void ChromeContentRendererClient::RenderFrameCreated(
     content::RenderFrame* render_frame) {
   new ChromeRenderFrameObserver(render_frame);
 
+  bool should_whitelist_for_content_settings =
+      CommandLine::ForCurrentProcess()->HasSwitch(switches::kInstantProcess);
   extensions::Dispatcher* ext_dispatcher = NULL;
 #if defined(ENABLE_EXTENSIONS)
   ext_dispatcher = extension_dispatcher_.get();
 #endif
-  ContentSettingsObserver* content_settings =
-      new ContentSettingsObserver(render_frame, ext_dispatcher);
+  ContentSettingsObserver* content_settings = new ContentSettingsObserver(
+      render_frame, ext_dispatcher, should_whitelist_for_content_settings);
   if (chrome_observer_.get()) {
     content_settings->SetContentSettingRules(
         chrome_observer_->content_setting_rules());

chrome/renderer/content_settings_observer.cc

@@ -6,7 +6,6 @@
 
 #include "base/command_line.h"
 #include "base/metrics/histogram.h"
-#include "chrome/common/chrome_switches.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/renderer/document_state.h"
@@ -150,7 +149,8 @@ ContentSetting GetContentSettingFromRules(
 
 ContentSettingsObserver::ContentSettingsObserver(
     content::RenderFrame* render_frame,
-    extensions::Dispatcher* extension_dispatcher)
+    extensions::Dispatcher* extension_dispatcher,
+    bool should_whitelist)
     : content::RenderFrameObserver(render_frame),
       content::RenderFrameObserverTracker<ContentSettingsObserver>(
           render_frame),
@@ -162,7 +162,8 @@ ContentSettingsObserver::ContentSettingsObserver(
       content_setting_rules_(NULL),
       is_interstitial_page_(false),
       npapi_plugins_blocked_(false),
-      current_request_id_(0) {
+      current_request_id_(0),
+      should_whitelist_(should_whitelist) {
   ClearBlockedContentSettings();
   render_frame->GetWebFrame()->setPermissionClient(this);
 
@@ -312,7 +313,7 @@ bool ContentSettingsObserver::allowImage(bool enabled_per_settings,
     if (is_interstitial_page_)
       return true;
 
-    if (IsWhitelistedForContentSettings(render_frame()))
+    if (IsWhitelistedForContentSettings())
       return true;
 
     if (content_setting_rules_) {
@@ -370,7 +371,7 @@ bool ContentSettingsObserver::allowScript(bool enabled_per_settings) {
         GURL(frame->document().securityOrigin().toString()));
     allow = setting != CONTENT_SETTING_BLOCK;
   }
-  allow = allow || IsWhitelistedForContentSettings(render_frame());
+  allow = allow || IsWhitelistedForContentSettings();
 
   cached_script_permissions_[frame] = allow;
   return allow;
@@ -392,7 +393,7 @@ bool ContentSettingsObserver::allowScriptFromSource(
                                    GURL(script_url));
     allow = setting != CONTENT_SETTING_BLOCK;
   }
-  return allow || IsWhitelistedForContentSettings(render_frame());
+  return allow || IsWhitelistedForContentSettings();
 }
 
 bool ContentSettingsObserver::allowStorage(bool local) {
@@ -674,18 +675,16 @@ const extensions::Extension* ContentSettingsObserver::GetExtension(
 }
 #endif
 
-bool ContentSettingsObserver::IsWhitelistedForContentSettings(
-    content::RenderFrame* frame) {
-  // Whitelist Instant processes.
-  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kInstantProcess))
+bool ContentSettingsObserver::IsWhitelistedForContentSettings() const {
+  if (should_whitelist_)
     return true;
 
   // Whitelist ftp directory listings, as they require JavaScript to function
   // properly.
-  if (frame->IsFTPDirectoryListing())
+  if (render_frame()->IsFTPDirectoryListing())
     return true;
 
-  WebFrame* web_frame = frame->GetWebFrame();
+  WebFrame* web_frame = render_frame()->GetWebFrame();
   return IsWhitelistedForContentSettings(web_frame->document().securityOrigin(),
                                          web_frame->document().url());
 }

chrome/renderer/content_settings_observer.h

@@ -33,8 +33,11 @@ class ContentSettingsObserver
       public content::RenderFrameObserverTracker<ContentSettingsObserver>,
       public blink::WebPermissionClient {
  public:
+  // Set |should_whitelist| to true if |render_frame()| contains content that
+  // should be whitelisted for content settings.
   ContentSettingsObserver(content::RenderFrame* render_frame,
-                          extensions::Dispatcher* extension_dispatcher);
+                          extensions::Dispatcher* extension_dispatcher,
+                          bool should_whitelist);
   ~ContentSettingsObserver() override;
 
   // Sets the content setting rules which back |AllowImage()|, |AllowScript()|,
@@ -119,8 +122,9 @@ class ContentSettingsObserver
 #endif
 
   // Helpers.
-  // True if |frame| contains content that is white-listed for content settings.
-  static bool IsWhitelistedForContentSettings(content::RenderFrame* frame);
+  // True if |render_frame()| contains content that is white-listed for content
+  // settings.
+  bool IsWhitelistedForContentSettings() const;
   static bool IsWhitelistedForContentSettings(
       const blink::WebSecurityOrigin& origin,
       const GURL& document_url);
@@ -158,6 +162,9 @@ class ContentSettingsObserver
   typedef std::map<int, blink::WebPermissionCallbacks> PermissionRequestMap;
   PermissionRequestMap permission_requests_;
 
+  // If true, IsWhitelistedForContentSettings will always return true.
+  const bool should_whitelist_;
+
   DISALLOW_COPY_AND_ASSIGN(ContentSettingsObserver);
 };
 

chrome/renderer/content_settings_observer_browsertest.cc

@@ -34,7 +34,7 @@ class MockContentSettingsObserver : public ContentSettingsObserver {
 
 MockContentSettingsObserver::MockContentSettingsObserver(
     content::RenderFrame* render_frame)
-    : ContentSettingsObserver(render_frame, NULL),
+    : ContentSettingsObserver(render_frame, NULL, false),
       image_url_("http://www.foo.com/image.jpg"),
       image_origin_("http://www.foo.com") {
 }