ifdef extensions bits in ContentSettingsObserver.

chrome/renderer/content_settings_observer.cc

Index: chrome/renderer/content_settings_observer.cc
diff --git a/chrome/renderer/content_settings_observer.cc b/chrome/renderer/content_settings_observer.cc
index ba6eb9eb9274e90464924162befb1aad415803fc..70734d08367b36314019e04de25cc67c7166b1ad 100644
--- a/chrome/renderer/content_settings_observer.cc
+++ b/chrome/renderer/content_settings_observer.cc
@@ -13,10 +13,6 @@
 #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"
@@ -28,6 +24,10 @@
 
 #if defined(ENABLE_EXTENSIONS)
 #include "chrome/common/extensions/chrome_extension_messages.h"
+#include "extensions/common/constants.h"
+#include "extensions/common/extension.h"
+#include "extensions/common/permissions/permissions_data.h"

[Devlin, 2014/08/12 15:34:59]

Doesn't actually look like we use this at first glance.

[Lei Zhang, 2014/08/12 23:03:17]

deleted

+#include "extensions/renderer/dispatcher.h"
 #endif
 
 using blink::WebDataSource;
@@ -40,7 +40,6 @@ using blink::WebURL;
 using blink::WebView;
 using content::DocumentState;
 using content::NavigationState;
-using extensions::APIPermission;
 
 namespace {
 
@@ -154,7 +153,9 @@ ContentSettingsObserver::ContentSettingsObserver(
     : 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),
@@ -429,19 +430,25 @@ bool ContentSettingsObserver::allowWriteToClipboard(bool default_value) {
 }
 
 bool ContentSettingsObserver::allowMutationEvents(bool default_value) {
+#if defined(ENABLE_EXTENSIONS)
   WebFrame* frame = render_frame()->GetWebFrame();
   WebSecurityOrigin origin = frame->document().securityOrigin();
   const extensions::Extension* extension = GetExtension(origin);
   if (extension && extension->is_platform_app())
     return false;
+#endif
   return default_value;
 }
 
 bool ContentSettingsObserver::allowPushState() {
+#if defined(ENABLE_EXTENSIONS)
   WebFrame* frame = render_frame()->GetWebFrame();

[Devlin, 2014/08/12 15:34:59]

This and the code in allowMutationEvents looks very similar, and it also looks
like we only use GetExtension(origin) in these two places.  What do you think of
refactoring so that GetExtension becomes HasExtensionAndIsPlatformApp()
(strawman name - feel free to use something better) and also does the work in
lines 434 - 438?  I'd like to try and keep these if defined blocks as
concentrated as possible...

[Lei Zhang, 2014/08/12 23:03:17]

[fry_squint] These two functions look similar.[/fry_squint]

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

:D

   WebSecurityOrigin origin = frame->document().securityOrigin();
   const extensions::Extension* extension = GetExtension(origin);
   return !extension || !extension->is_platform_app();
+#else
+  return true;
+#endif
 }
 
 static void SendInsecureContentSignal(int signal) {
@@ -635,6 +642,7 @@ void ContentSettingsObserver::ClearBlockedContentSettings() {
   cached_script_permissions_.clear();
 }
 
+#if defined(ENABLE_EXTENSIONS)
 const extensions::Extension* ContentSettingsObserver::GetExtension(
     const WebSecurityOrigin& origin) const {
   if (!EqualsASCII(origin.protocol(), extensions::kExtensionScheme))
@@ -646,6 +654,7 @@ const extensions::Extension* ContentSettingsObserver::GetExtension(
 
   return extension_dispatcher_->extensions()->GetByID(extension_id);
 }
+#endif
 
 bool ContentSettingsObserver::IsWhitelistedForContentSettings(
     content::RenderFrame* frame) {
@@ -678,8 +687,10 @@ bool ContentSettingsObserver::IsWhitelistedForContentSettings(
   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.