[Extensions] Make chrome://extensions use developerPrivate for inspect

chrome/browser/extensions/api/developer_private/developer_private_api.cc

Index: chrome/browser/extensions/api/developer_private/developer_private_api.cc
diff --git a/chrome/browser/extensions/api/developer_private/developer_private_api.cc b/chrome/browser/extensions/api/developer_private/developer_private_api.cc
index 9470e7821be80f10b1f9df841071588ddfea849e..f94a5aa39b8719efafd927006d8aed9b9c48be15 100644
--- a/chrome/browser/extensions/api/developer_private/developer_private_api.cc
+++ b/chrome/browser/extensions/api/developer_private/developer_private_api.cc
@@ -100,6 +100,9 @@ const char kCouldNotShowSelectFileDialogError[] =
     "Could not show a file chooser.";
 const char kFileSelectionCanceled[] =
     "File selection was canceled.";
+const char kInvalidRenderViewIdError[] = "Invalid render process id.";
+const char kInvalidRenderProcessIdError[] = "Invalid render process id.";

[not at google - send to devlin, 2015/03/09 18:26:38]

These say the same thing, though I don't know if I'd bother having 2 errors
anyway.

[Devlin, 2015/03/09 19:13:57]

Done.

+const char kNoSuchRendererError[] = "No such renderer.";
 
 const char kUnpackedAppsFolder[] = "apps_target";
 
@@ -836,39 +839,45 @@ DeveloperPrivateShowPermissionsDialogFunction::
 DeveloperPrivateShowPermissionsDialogFunction::
     ~DeveloperPrivateShowPermissionsDialogFunction() {}
 
-bool DeveloperPrivateInspectFunction::RunSync() {
+ExtensionFunction::ResponseAction DeveloperPrivateInspectFunction::Run() {
   scoped_ptr<developer::Inspect::Params> params(
       developer::Inspect::Params::Create(*args_));
-  EXTENSION_FUNCTION_VALIDATE(params.get() != NULL);
+  EXTENSION_FUNCTION_VALIDATE(params);
   const developer::InspectOptions& options = params->options;
 
-  int render_process_id;
-  base::StringToInt(options.render_process_id, &render_process_id);
+  // TODO(devlin): These should clearly not be strings.

[not at google - send to devlin, 2015/03/09 18:26:38]

Hmm well what you could do is change the IDL to be a choice[*]. That will
generate options.render_process_id. as_string and
options.render_process_id.as_integer. The code could then be:

if (options.render_process_id.as_string &&
    !base::StringToInt(*options.render_process_id.as_string,
                       options.render_process_id.as_integer) {
  return RespondNow(Error(...));
}

or, whatever you desire - but then the chrome://extensions code can actually
make sense.

[*] I think the syntax is:
(DOMString | integer) render_process_id;

[Devlin, 2015/03/09 19:13:57]

Nifty. Done.

+  int render_process_id = 0;
+  if (!base::StringToInt(options.render_process_id, &render_process_id))
+    return RespondNow(Error(kInvalidRenderProcessIdError));
+  int render_view_id = 0;
+  if (!base::StringToInt(options.render_view_id, &render_view_id))
+    return RespondNow(Error(kInvalidRenderViewIdError));
 
   if (render_process_id == -1) {
-    // This is a lazy background page. Identify if it is a normal
-    // or incognito background page.
+    // This is a lazy background page.
     const Extension* extension = ExtensionRegistry::Get(
-        GetProfile())->enabled_extensions().GetByID(options.extension_id);
-    DCHECK(extension);
-    // Wakes up the background page and  opens the inspect window.
-    devtools_util::InspectBackgroundPage(extension, GetProfile());
-    return false;
+        browser_context())->enabled_extensions().GetByID(options.extension_id);

[not at google - send to devlin, 2015/03/09 18:26:38]

GetExtensionById, or a version of that which only looks at enabled extensions?

[Devlin, 2015/03/09 19:13:57]

Hmm... the point is to be briefer than ExtensionRegistry::GetExtensionById.  If
we start adding bools and masks, doesn't that kinda, well, eliminate that point?
 Is it really worth it for one line?

[not at google - send to devlin, 2015/03/09 21:13:17]

to be consistent with ExtensionRegistry, which takes a flag:

https://code.google.com/p/chromium/codesearch#chromium/src/extensions/browser...

but whatevs.

+    if (!extension)
+      return RespondNow(Error(kNoSuchExtensionError));
+
+    Profile* profile = Profile::FromBrowserContext(browser_context());
+    if (options.incognito)
+      profile = profile->GetOffTheRecordProfile();
+
+    // Wakes up the background page and opens the inspect window.
+    devtools_util::InspectBackgroundPage(extension, profile);
+    return RespondNow(NoArguments());
   }
 
-  int render_view_id;
-  base::StringToInt(options.render_view_id, &render_view_id);
   content::RenderViewHost* host = content::RenderViewHost::FromID(
       render_process_id, render_view_id);
 
-  if (!host || !content::WebContents::FromRenderViewHost(host)) {
-    // This can happen if the host has gone away since the page was displayed.
-    return false;
-  }
+  if (!host || !content::WebContents::FromRenderViewHost(host))
+    return RespondNow(Error(kNoSuchRendererError));
 
   DevToolsWindow::OpenDevToolsWindow(
       content::WebContents::FromRenderViewHost(host));
-  return true;
+  return RespondNow(NoArguments());
 }
 
 DeveloperPrivateInspectFunction::~DeveloperPrivateInspectFunction() {}