Enable devtools via context menu in platform apps and popup extensions.

chrome/browser/tab_contents/render_view_context_menu.cc

Index: chrome/browser/tab_contents/render_view_context_menu.cc
diff --git a/chrome/browser/tab_contents/render_view_context_menu.cc b/chrome/browser/tab_contents/render_view_context_menu.cc
index 2fd71bb145dac9c151497cc18109e55d7b925948..e43ba5981e446d0b5dd439f1d52cb210ec57d7fd 100644
--- a/chrome/browser/tab_contents/render_view_context_menu.cc
+++ b/chrome/browser/tab_contents/render_view_context_menu.cc
@@ -630,7 +630,7 @@ void RenderViewContextMenu::InitMenu() {
   observers_.AddObserver(print_preview_menu_observer_.get());
 }
 
-const Extension* RenderViewContextMenu::GetPlatformApp() const {
+const Extension* RenderViewContextMenu::GetExtension() const {
   ExtensionProcessManager* process_manager =
       profile_->GetExtensionProcessManager();
   // There is no process manager in some tests.
@@ -643,8 +643,10 @@ const Extension* RenderViewContextMenu::GetPlatformApp() const {
        ++iter) {
     ExtensionHost* host = *iter;
     if (host->host_contents() == source_web_contents_) {
-      if (host->extension() && host->extension()->is_platform_app()) {
+      if (host->extension()) {
         return host->extension();
+      } else {
+        return NULL;
       }

[not at google - send to devlin, 2012/01/23 05:43:51]

nit sort of stuff:

Might be nice to save a reference to Extension* here.

Also, my personal preference for these sorts of things is to leave out the
"else" clause and just have "return NULL".

So like

const Extension* extension = host->extension();
if (extension)
  return extension;
return NULL;

[benwells, 2012/01/23 06:42:23]

Done.

     }
   }
@@ -652,11 +654,24 @@ const Extension* RenderViewContextMenu::GetPlatformApp() const {
   return NULL;
 }
 
+const Extension* RenderViewContextMenu::GetPlatformApp() const {
+  const Extension* extension = GetExtension();
+  if (extension && extension->is_platform_app()) {
+    return extension;
+  } else {
+    return NULL;
+  }
+}
+
 void RenderViewContextMenu::AppendPlatformAppItems() {

[not at google - send to devlin, 2012/01/23 05:43:51]

how about passing in the Extension*, save needing to call GetPlatformApp again.

[benwells, 2012/01/23 06:42:23]

Done.

   const Extension* platform_app = GetPlatformApp();
   DCHECK(platform_app);
   int index = 0;
   AppendExtensionItems(platform_app->id(), &index);
+
+  // Add dev tools for unpacked extensions.
+  if (platform_app->location() == Extension::LOAD)
+    AppendDeveloperItems();
 }
 
 void RenderViewContextMenu::LookUpInDictionary() {
@@ -1862,17 +1877,23 @@ void RenderViewContextMenu::MenuClosed(ui::SimpleMenuModel* source) {
 
 bool RenderViewContextMenu::IsDevCommandEnabled(int id) const {
   if (id == IDC_CONTENT_CONTEXT_INSPECTELEMENT) {
-    const CommandLine& command_line = *CommandLine::ForCurrentProcess();
-    TabContentsWrapper* tab_contents_wrapper =
-        TabContentsWrapper::GetCurrentWrapperForContents(
-            source_web_contents_);
-    if (!tab_contents_wrapper)
-      return false;
-    // Don't enable the web inspector if JavaScript is disabled.
-    if (!tab_contents_wrapper->prefs_tab_helper()->per_tab_prefs()->GetBoolean(
-            prefs::kWebKitJavascriptEnabled) ||
-        command_line.HasSwitch(switches::kDisableJavaScript))
-      return false;
+    // Don't enable the web inspector if JavaScript is disabled. We don't
+    // check this when this is the web contents of an extension (e.g.
+    // for a popup extension or a platform app) as they have javascript
+    // always enabled.
+    if (!GetExtension()) {
+      const CommandLine& command_line = *CommandLine::ForCurrentProcess();

[not at google - send to devlin, 2012/01/23 05:43:51]

I'd move this down to right before it's used. Also... just store it locally as a
pointer rather than a reference... eh.

[benwells, 2012/01/23 06:42:23]

Ya much nicer. Done.

+      TabContentsWrapper* contents_wrapper =
+          TabContentsWrapper::GetCurrentWrapperForContents(
+              source_web_contents_);
+      if (!contents_wrapper)
+        return false;
+      if (!contents_wrapper->prefs_tab_helper()->per_tab_prefs()->GetBoolean(
+              prefs::kWebKitJavascriptEnabled) ||

[not at google - send to devlin, 2012/01/23 05:43:51]

<- 2 indent

[benwells, 2012/01/23 06:42:23]

I think it is right as is? There is an extra four space indent for the
GetBoolean call... is that the rule?

+          command_line.HasSwitch(switches::kDisableJavaScript))
+        return false;
+    }
+
     // Don't enable the web inspector if the developer tools are disabled via
     // the preference dev-tools-disabled.
     if (profile_->GetPrefs()->GetBoolean(prefs::kDevToolsDisabled))