First few API implementation of AppsDebuggerPrivate.

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
new file mode 100644
index 0000000000000000000000000000000000000000..1f8ae54a5964b62ec9769a7f93a4f89f15546dd1
--- /dev/null
+++ b/chrome/browser/extensions/api/developer_private/developer_private_api.cc
@@ -0,0 +1,268 @@
+// 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/browser/extensions/api/developer_private/developer_private_api.h"
+
+#include "base/string_number_conversions.h"
+#include "base/values.h"
+#include "chrome/browser/debugger/devtools_window.h"
+#include "chrome/browser/extensions/api/developer_private/developer_private_api_factory.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_system.h"
+#include "chrome/browser/extensions/management_policy.h"
+#include "chrome/browser/extensions/updater/extension_updater.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/webui/extensions/extension_icon_source.h"
+#include "chrome/browser/view_type_utils.h"
+#include "chrome/common/extensions/api/developer_private.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/web_contents.h"
+#include "grit/generated_resources.h"
+
+using content::RenderViewHost;
+using extensions::DeveloperPrivateAPI;
+using extensions::Extension;
+using extensions::ExtensionSystem;
+
+namespace {
+
+extensions::ExtensionUpdater* GetExtensionUpdater(Profile* profile) {
+    return profile->GetExtensionService()->updater();
+}
+
+} // namespace

[miket_OOO, 2012/12/06 19:31:15]

always two spaces before //

(please fix all instances; I'll stop repeating myself)

+
+namespace extensions {
+
+DeveloperPrivateAPI* DeveloperPrivateAPI::Get(Profile* profile) {
+  return DeveloperPrivateAPIFactory::GetForProfile(profile);
+}
+
+DeveloperPrivateAPI::DeveloperPrivateAPI(Profile* profile)
+  : profile_(profile),
+    deleting_rvh_(NULL) {
+      RegisterNotifications();
+}
+
+scoped_ptr<developer::ItemInfo> DeveloperPrivateAPI::CreateItemInfo(
+    const Extension& item,
+    ExtensionSystem* system,
+    bool item_is_enabled) {
+  scoped_ptr<developer::ItemInfo> info(new developer::ItemInfo());
+  ExtensionService* service = system->extension_service();
+
+  info->id = item.id();
+  info->name = item.name();
+  info->enabled = service->IsExtensionEnabled(info->id);
+  info->offline_enabled = item.offline_enabled();
+  info->version = item.VersionString();
+  info->description = item.description();
+
+  if (item.is_app()) {
+    if (item.is_legacy_packaged_app())
+      info->type = developer::DEVELOPER_PRIVATE_ITEM_TYPE_LEGACY_PACKAGED_APP;
+    else if (item.is_hosted_app())
+      info->type = developer::DEVELOPER_PRIVATE_ITEM_TYPE_HOSTED_APP;
+    else
+      info->type = developer::DEVELOPER_PRIVATE_ITEM_TYPE_PACKAGED_APP;
+  } else if (item.is_theme()) {
+    info->type = developer::DEVELOPER_PRIVATE_ITEM_TYPE_THEME;
+  } else {
+    info->type = developer::DEVELOPER_PRIVATE_ITEM_TYPE_EXTENSION;
+  }

[miket_OOO, 2012/12/06 19:31:15]

Consider some defensive checks here that your catchall else cases are correct.
If someone adds a new type in the future, they might not know about this code,
and suddenly (and silently) this API will break. I'm thinking something like
this:

CHECK(item.is_packaged_app()) in the first else case
CHECK(item.is_extension()) in the second case.

Or else do this

if (foo) {
} else if (bar) {
} else if (baz) {
} else {
NOTREACHED();
}

[Gaurav, 2012/12/12 02:23:38]

Done.

+
+  if (item.location() == Extension::LOAD) {
+    info->path.reset(new std::string(item.path().value()));
+  }
+
+  info->enabled_incognito = service->IsIncognitoEnabled(item.id());
+  info->wants_file_access = item.wants_file_access();
+  info->allow_file_access = service->AllowFileAccess(&item);
+  info->allow_reload = (item.location() == Extension::LOAD);
+  info->is_unpacked = (item.location() == Extension::LOAD);
+
+  GURL icon =
+      ExtensionIconSource::GetIconURL(&item,
+                                      extension_misc::EXTENSION_ICON_MEDIUM,
+                                      ExtensionIconSet::MATCH_BIGGER,
+                                      !info->enabled,
+                                      NULL);
+  info->icon = icon.spec();
+
+  info->homepage_url.reset(new std::string(item.GetHomepageURL().spec()));
+  if (!item.options_url().is_empty()) {
+    info->options_url.reset(new std::string(item.options_url().spec()));
+  }
+
+  if (!item.update_url().is_empty()) {
+    info->update_url.reset(new std::string(
+        item.update_url().spec()));

[miket_OOO, 2012/12/06 19:31:15]

is this wrapping necessary?

[Gaurav, 2012/12/12 02:23:38]

update_url is optional parameter and is a string pointer.

On 2012/12/06 19:31:15, miket wrote:

+  }
+
+  if (item.is_app()) {
+    info->app_launch_url.reset(new std::string(
+        item.GetFullLaunchURL().spec()));
+  }
+
+  info->may_disable = system->management_policy()->
+      UserMayModifySettings(&item, NULL);
+  info->is_app = item.is_app();
+  info->views = GetInspectablePagesForExtension(&item, item_is_enabled);
+
+  return info.Pass();
+}
+
+void DeveloperPrivateAPI::AddItemsInfo(const ExtensionSet& items,
+                                       ExtensionSystem* system,
+                                       ItemInfoList* item_list) {
+
+  for (ExtensionSet::const_iterator iter = items.begin();
+       iter != items.end(); ++iter) {
+    const Extension& item = **iter;
+    if (item.location() == Extension::COMPONENT)
+      continue; // Skip built-in extensions / apps;
+    item_list->push_back(make_linked_ptr<developer::ItemInfo>(
+        CreateItemInfo(item, system, false).release()));
+  }
+}
+
+void DeveloperPrivateAPI::GetInspectablePagesForExtensionProcess(
+    const std::set<content::RenderViewHost*>& views,
+    ItemInspectViewList* result) {
+  for (std::set<content::RenderViewHost*>::const_iterator iter = views.begin();
+       iter != views.end(); ++iter) {
+    content::RenderViewHost* host = *iter;
+    content::WebContents* web_contents =
+        content::WebContents::FromRenderViewHost(host);
+    chrome::ViewType host_type = chrome::GetViewType(web_contents);
+    if (host == deleting_rvh_ ||
+        chrome::VIEW_TYPE_EXTENSION_POPUP == host_type ||
+        chrome::VIEW_TYPE_EXTENSION_DIALOG == host_type)
+      continue;
+
+    GURL url = web_contents->GetURL();
+    content::RenderProcessHost* process = host->GetProcess();
+    linked_ptr<developer::ItemInspectView>
+        view(new developer::ItemInspectView());
+    view->path = url.path().substr(1);
+    view->render_process_id = process->GetID();
+    view->render_view_id = host->GetRoutingID();
+    view->incognito = process->GetBrowserContext()->IsOffTheRecord();
+
+    result->push_back(view);
+  }
+}
+
+ItemInspectViewList DeveloperPrivateAPI::GetInspectablePagesForExtension(
+    const extensions::Extension* extension,
+    bool extension_is_enabled) {
+
+  ItemInspectViewList result;
+  // Get the extension process's active views.
+  ExtensionProcessManager* process_manager =
+      extensions::ExtensionSystem::Get(profile_)->process_manager();
+  GetInspectablePagesForExtensionProcess(
+      process_manager->GetRenderViewHostsForExtension(extension->id()),
+      &result);
+  return result;
+}
+
+void DeveloperPrivateAPI::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  Profile* source_profile = NULL;
+  switch (type) {
+    // TODO(grv): Listen to other notifications.
+    case content::NOTIFICATION_RENDER_VIEW_HOST_DELETED:
+    case chrome::NOTIFICATION_BACKGROUND_CONTENTS_DELETED:
+      deleting_rvh_ = content::Source<RenderViewHost>(source).ptr();
+      source_profile = content::Source<Profile>(source).ptr();
+      break;
+    default:
+      NOTREACHED();
+  }
+}
+
+void DeveloperPrivateAPI::RegisterNotifications() {
+  registrar_.Add(this,
+                 content::NOTIFICATION_RENDER_VIEW_HOST_DELETED,
+                 content::NotificationService::AllBrowserContextsAndSources());
+  registrar_.Add(this,
+                 chrome::NOTIFICATION_BACKGROUND_CONTENTS_DELETED,
+                 content::NotificationService::AllBrowserContextsAndSources());
+
+}
+
+DeveloperPrivateAPI::~DeveloperPrivateAPI() {
+}
+
+void DeveloperPrivateAPI::Shutdown() {
+}
+
+namespace api {
+
+bool DeveloperPrivateAutoUpdateFunction::RunImpl() {
+  extensions::ExtensionUpdater* updater = GetExtensionUpdater(profile());
+  if (updater)
+    updater->CheckNow(extensions::ExtensionUpdater::CheckParams());
+  SetResult(Value::CreateBooleanValue(true));
+  return true;
+}
+
+bool DeveloperPrivateGetItemsInfoFunction::RunImpl() {
+  ItemInfoList items;
+  ExtensionSystem* system = ExtensionSystem::Get(profile());
+
+  DeveloperPrivateAPI::Get(profile())->AddItemsInfo(
+      *profile()->GetExtensionService()->extensions(), system, &items);
+
+  results_ = developer::GetItemsInfo::Results::Create(items);
+  return true;
+}
+
+bool DeveloperPrivateInspectFunction::RunImpl() {
+  scoped_ptr<developer::Inspect::Params> params(
+      developer::Inspect::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get() != NULL);
+  const developer::InspectOptions& options = params->options;
+  std::string extension_id;

[miket_OOO, 2012/12/06 19:31:15]

These declarations could be closer to where they're used.

[Gaurav, 2012/12/12 02:23:38]

Done.

+  int render_process_id;
+  int render_view_id;
+  bool incognito = false;
+
+  extension_id = options.extension_id;
+  base::StringToInt(options.render_process_id, &render_process_id);
+  base::StringToInt(options.render_view_id, &render_view_id);
+    incognito = options.incognito;

[miket_OOO, 2012/12/06 19:31:15]

indentation

[Gaurav, 2012/12/12 02:23:38]

Done.

+
+  if (render_process_id == -1) {

[miket_OOO, 2012/12/06 19:31:15]

Not a big deal, but why convert render_view_id to an int if you already know
that you won't need it?

[Gaurav, 2012/12/12 02:23:38]

Done.

+    // This message is for a lazy background page. Start the page if necessary.
+    ExtensionService* service = profile()->GetExtensionService();
+    const Extension* extension = service->extensions()->GetByID(extension_id);
+    DCHECK(extension);
+
+    if (incognito)

[miket_OOO, 2012/12/06 19:31:15]

Is this logic out of order? Are you asking the non-incognito service for the
extension in the incognito case?

I asked asargent about this, and he wasn't sure whether it was even necessary to
get the incognito service separately. Can you please confirm that this is
correct, and if it is, put some kind of comment in here to let people know why
it's weird?

[Gaurav, 2012/12/12 02:23:38]

The lazy background pages may return -1 as render_process_id. In that case, we
identify the correct service normal/incognito and wake up the page.
On 2012/12/06 19:31:15, miket wrote:

+      service = extensions::ExtensionSystem::Get(
+          service->profile()->GetOffTheRecordProfile())->extension_service();
+    service->InspectBackgroundPage(extension);
+    return false;
+  }
+
+  content::RenderViewHost* host = content::RenderViewHost::FromID(
+      render_process_id, render_view_id);
+
+  if (!host) {
+    // This can happen if the host has gone away since the page was dispalyed.

[miket_OOO, 2012/12/06 19:31:15]

spelling

[Gaurav, 2012/12/12 02:23:38]

Done.

+    return false;
+  }
+
+  DevToolsWindow::OpenDevToolsWindow(host);
+  return true;
+}
+
+} // namespace api

[miket_OOO, 2012/12/06 19:31:15]

vertical whitespace (please fix all)

[Gaurav, 2012/12/12 02:23:38]

Done.

+} // namespace extensions