ifdef extensions bits in ContentSettingsObserver.

chrome/renderer/content_settings_observer.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/renderer/content_settings_observer.h"
 
 #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"
 #include "content/public/renderer/navigation_state.h"
 #include "content/public/renderer/render_frame.h"
 #include "content/public/renderer/render_view.h"
-#include "extensions/common/constants.h"
-#include "extensions/common/extension.h"
-#include "extensions/common/permissions/permissions_data.h"
-#include "extensions/renderer/dispatcher.h"
 #include "third_party/WebKit/public/platform/WebPermissionCallbacks.h"
 #include "third_party/WebKit/public/platform/WebURL.h"
 #include "third_party/WebKit/public/web/WebDataSource.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebFrame.h"
 #include "third_party/WebKit/public/web/WebFrameClient.h"
 #include "third_party/WebKit/public/web/WebSecurityOrigin.h"
 #include "third_party/WebKit/public/web/WebView.h"
 
 #if defined(ENABLE_EXTENSIONS)
 #include "chrome/common/extensions/chrome_extension_messages.h"
+#include "extensions/common/constants.h"
+#include "extensions/common/extension.h"
+#include "extensions/renderer/dispatcher.h"
 #endif
 
 using blink::WebDataSource;
 using blink::WebDocument;
 using blink::WebFrame;
 using blink::WebPermissionCallbacks;
 using blink::WebSecurityOrigin;
 using blink::WebString;
 using blink::WebURL;
 using blink::WebView;
 using content::DocumentState;
 using content::NavigationState;
-using extensions::APIPermission;
 
 namespace {
 
 enum {
   INSECURE_CONTENT_DISPLAY = 0,
   INSECURE_CONTENT_DISPLAY_HOST_GOOGLE,
   INSECURE_CONTENT_DISPLAY_HOST_WWW_GOOGLE,
   INSECURE_CONTENT_DISPLAY_HTML,
   INSECURE_CONTENT_RUN,
   INSECURE_CONTENT_RUN_HOST_GOOGLE,
@@ skipping 93 matching lines @@
 }
 
 }  // namespace
 
 ContentSettingsObserver::ContentSettingsObserver(
     content::RenderFrame* render_frame,
     extensions::Dispatcher* extension_dispatcher)
     : content::RenderFrameObserver(render_frame),
       content::RenderFrameObserverTracker<ContentSettingsObserver>(
           render_frame),
+#if defined(ENABLE_EXTENSIONS)
       extension_dispatcher_(extension_dispatcher),
+#endif
       allow_displaying_insecure_content_(false),
       allow_running_insecure_content_(false),
       content_setting_rules_(NULL),
       is_interstitial_page_(false),
       npapi_plugins_blocked_(false),
       current_request_id_(0) {
   ClearBlockedContentSettings();
   render_frame->GetWebFrame()->setPermissionClient(this);
 
   if (render_frame->GetRenderView()->GetMainRenderFrame() != render_frame) {
@@ skipping 254 matching lines @@
   bool allowed = false;
 #if defined(ENABLE_EXTENSIONS)
   WebFrame* frame = render_frame()->GetWebFrame();
   Send(new ChromeViewHostMsg_CanTriggerClipboardWrite(
       GURL(frame->document().securityOrigin().toString()), &allowed));
 #endif
   return allowed;
 }
 
 bool ContentSettingsObserver::allowMutationEvents(bool default_value) {
-  WebFrame* frame = render_frame()->GetWebFrame();
-  WebSecurityOrigin origin = frame->document().securityOrigin();
-  const extensions::Extension* extension = GetExtension(origin);
-  if (extension && extension->is_platform_app())
-    return false;
-  return default_value;
+  return IsPlatformApp() ? false : default_value;
 }
 
 bool ContentSettingsObserver::allowPushState() {
-  WebFrame* frame = render_frame()->GetWebFrame();
-  WebSecurityOrigin origin = frame->document().securityOrigin();
-  const extensions::Extension* extension = GetExtension(origin);
-  return !extension || !extension->is_platform_app();
+  return !IsPlatformApp();
 }
 
 static void SendInsecureContentSignal(int signal) {
   UMA_HISTOGRAM_ENUMERATION("SSL.InsecureContent", signal,
                             INSECURE_CONTENT_NUM_EVENTS);
 }
 
 bool ContentSettingsObserver::allowDisplayingInsecureContent(
     bool allowed_per_settings,
     const blink::WebSecurityOrigin& origin,
@@ skipping 173 matching lines @@
   callbacks.doDeny();
 }
 
 void ContentSettingsObserver::ClearBlockedContentSettings() {
   for (size_t i = 0; i < arraysize(content_blocked_); ++i)
     content_blocked_[i] = false;
   cached_storage_permissions_.clear();
   cached_script_permissions_.clear();
 }
 
+bool ContentSettingsObserver::IsPlatformApp() {
+#if defined(ENABLE_EXTENSIONS)
+  WebFrame* frame = render_frame()->GetWebFrame();
+  WebSecurityOrigin origin = frame->document().securityOrigin();
+  const extensions::Extension* extension = GetExtension(origin);

[Devlin, 2014/08/12 23:16:12]

Any reason to not save a couple ifdefs and just inline GetExtension() here?

const Extension* = NULL;
if (EqualsASCII(origin.protocol(), extensions::kExtensionScheme)) {
  const std::string extension_id = origin.host().utf8().data();
  extension = extension_dispatcher_->IsExtensionActive(extension_id) ?
      extension_dispatcher_->extensions()->GetByID(extension_id) : NULL;
}

[Lei Zhang, 2014/08/12 23:18:06]

It's already there and it's very obvious what it does?

[Devlin, 2014/08/12 23:20:47]

Yeah.  I just think that ifdefs in general make code harder to read (and harder
to pull out functions that shouldn't be there), so the more we condense them the
better.  But... I don't feel strongly enough about it in this case. :)

+  return extension && extension->is_platform_app();
+#else
+  return false;
+#endif
+}
+
+#if defined(ENABLE_EXTENSIONS)
 const extensions::Extension* ContentSettingsObserver::GetExtension(
     const WebSecurityOrigin& origin) const {
   if (!EqualsASCII(origin.protocol(), extensions::kExtensionScheme))
     return NULL;
 
   const std::string extension_id = origin.host().utf8().data();
   if (!extension_dispatcher_->IsExtensionActive(extension_id))
     return NULL;
 
   return extension_dispatcher_->extensions()->GetByID(extension_id);
 }
+#endif
 
 bool ContentSettingsObserver::IsWhitelistedForContentSettings(
     content::RenderFrame* frame) {
   // Whitelist Instant processes.
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kInstantProcess))
     return true;
 
   // Whitelist ftp directory listings, as they require JavaScript to function
   // properly.
   if (frame->IsFTPDirectoryListing())
@@ skipping 12 matching lines @@
 
   if (origin.isUnique())
     return false;  // Uninitialized document?
 
   if (EqualsASCII(origin.protocol(), content::kChromeUIScheme))
     return true;  // Browser UI elements should still work.
 
   if (EqualsASCII(origin.protocol(), content::kChromeDevToolsScheme))
     return true;  // DevTools UI elements should still work.
 
+#if defined(ENABLE_EXTENSIONS)
   if (EqualsASCII(origin.protocol(), extensions::kExtensionScheme))
     return true;
+#endif
 
   // TODO(creis, fsamuel): Remove this once the concept of swapped out
   // RenderFrames goes away.
   if (document_url == GURL(content::kSwappedOutURL))
     return true;
 
   // If the scheme is file:, an empty file name indicates a directory listing,
   // which requires JavaScript to function properly.
   if (EqualsASCII(origin.protocol(), url::kFileScheme)) {
     return document_url.SchemeIs(url::kFileScheme) &&
            document_url.ExtractFileName().empty();
   }
 
   return false;
 }