Add a new field "source" in launchData of chrome.app.runtime.onLaunched() to trace launch source.

extensions/browser/api/app_runtime/app_runtime_api.cc

Index: extensions/browser/api/app_runtime/app_runtime_api.cc
diff --git a/extensions/browser/api/app_runtime/app_runtime_api.cc b/extensions/browser/api/app_runtime/app_runtime_api.cc
index 0704478dbeac084aa2b03cad4219f9a95a06c2a7..4f79d5f3f3e1798458fc370961552e8035c9448a 100644
--- a/extensions/browser/api/app_runtime/app_runtime_api.cc
+++ b/extensions/browser/api/app_runtime/app_runtime_api.cc
@@ -12,6 +12,8 @@
 #include "extensions/browser/extensions_browser_client.h"
 #include "extensions/browser/granted_file_entry.h"
 #include "extensions/common/api/app_runtime.h"
+#include "extensions/common/constants.h"
+#include "extensions/common/feature_switch.h"
 #include "url/gurl.h"
 
 using content::BrowserContext;
@@ -57,6 +59,41 @@ void DispatchOnLaunchedEventImpl(const std::string& extension_id,
       ->SetLastLaunchTime(extension_id, base::Time::Now());
 }
 
+app_runtime::LaunchSource getLaunchSourceEnum(
+    extensions::AppLaunchSource source) {
+  switch (source) {
+    case extensions::SOURCE_APP_LAUNCHER:

[benwells, 2014/10/28 21:22:48]

Can you do this with an array lookup instead?

[cylee1, 2014/10/29 16:39:59]

Could you give me an example? Do you mean defining something like
mapping = [extensions::SOURCE_APP_LAUNCHER, extensions::SOURCE_NEW_PAGE, ...]
and match it by mapping[source] ?
If so it's vulnerable to enum deletion/reordering/insertion in the middle, and
requires enum->int type conversion and boundary checking ("default:" case
here"), which I think make no much difference to a direct type conversion as
static_cast<app_runtime::LaunchSource>(source) since both way need to sync the
two enums manually.
This way is ugly but safe and I see others doing the same thing. Solutions
welcome.

[benwells, 2014/10/30 06:04:28]

OK fair enough.

+      return app_runtime::LAUNCH_SOURCE_APP_LAUNCHER;
+    case extensions::SOURCE_NEW_TAB_PAGE:
+      return app_runtime::LAUNCH_SOURCE_NEW_TAB_PAGE;
+    case extensions::SOURCE_RELOAD:
+      return app_runtime::LAUNCH_SOURCE_RELOAD;
+    case extensions::SOURCE_RESTART:
+      return app_runtime::LAUNCH_SOURCE_RESTART;
+    case extensions::SOURCE_LOAD_AND_LAUNCH:
+      return app_runtime::LAUNCH_SOURCE_LOAD_AND_LAUNCH;
+    case extensions::SOURCE_FILE_HANDLER:
+      return app_runtime::LAUNCH_SOURCE_FILE_HANDLER;
+    case extensions::SOURCE_URL_HANDLER:
+      return app_runtime::LAUNCH_SOURCE_URL_HANDLER;
+
+    case extensions::SOURCE_ATHENA:
+      return app_runtime::LAUNCH_SOURCE_ATHENA;
+    case extensions::SOURCE_WIN_METRO:
+      return app_runtime::LAUNCH_SOURCE_WIN_METRO;
+
+    case extensions::SOURCE_SYSTEM_TRAY:
+      return app_runtime::LAUNCH_SOURCE_SYSTEM_TRAY;
+    case extensions::SOURCE_ABOUT_PAGE:
+      return app_runtime::LAUNCH_SOURCE_ABOUT_PAGE;
+    case extensions::SOURCE_KEYBOARD:
+      return app_runtime::LAUNCH_SOURCE_KEYBOARD;
+
+    default:
+      return app_runtime::LAUNCH_SOURCE_NONE;
+  }
+}
+
 }  // namespace
 
 // static
@@ -71,9 +108,14 @@ void AppRuntimeEventRouter::DispatchOnEmbedRequestedEvent(
 // static
 void AppRuntimeEventRouter::DispatchOnLaunchedEvent(
     BrowserContext* context,
-    const Extension* extension) {
-  scoped_ptr<base::DictionaryValue> launch_data(new base::DictionaryValue());
-  DispatchOnLaunchedEventImpl(extension->id(), launch_data.Pass(), context);
+    const Extension* extension,
+    extensions::AppLaunchSource source) {
+  app_runtime::LaunchData launch_data;

[benwells, 2014/10/28 21:22:48]

It would be great to do some UMA logging here of the source.

[cylee1, 2014/10/29 16:40:00]

Added in DispatchOnLaunchedEventImpl to handle restart and file handler cases as
well.

+  if (extensions::FeatureSwitch::trace_app_source()->IsEnabled()) {
+    launch_data.source = getLaunchSourceEnum(source);
+  }
+  DispatchOnLaunchedEventImpl(
+      extension->id(), launch_data.ToValue().Pass(), context);
 }
 
 // static
@@ -97,20 +139,31 @@ void AppRuntimeEventRouter::DispatchOnLaunchedEventWithFileEntries(
     const std::vector<GrantedFileEntry>& file_entries) {
   // TODO(sergeygs): Use the same way of creating an event (using the generated
   // boilerplate) as below in DispatchOnLaunchedEventWithUrl.
-  scoped_ptr<base::DictionaryValue> launch_data(new base::DictionaryValue);
+  base::DictionaryValue* launch_data = new base::DictionaryValue();

[benwells, 2014/10/28 21:22:47]

Why change this?

[cylee1, 2014/10/29 16:39:59]

Because the ownership is meant to be transferred when calling
DispatchOnLaunchedEventImpl (just realized I don't need to call Pass() at the
caller according to
http://www.chromium.org/developers/smart-pointer-guidelines), so IMHO it's not
necessary to use scoped_ptr here. Is there any convention that we should always
use smart pointer when possible?

It's especially true for the variable "items", which was declared as a scope_ptr
and then items.release() is called soon after. In the case I see no need to use
smart pointer ?

[benwells, 2014/10/30 06:04:28]

If you want to change that please do it in another patch, it makes your actual
change harder to see.

[cylee1, 2014/10/30 13:03:43]

Done.

   launch_data->SetString("id", handler_id);
-  scoped_ptr<base::ListValue> items(new base::ListValue);
+  launch_data->SetString(

[benwells, 2014/10/28 21:22:48]

Shouldn't this be gated on the feature switch too?

[cylee1, 2014/10/29 16:40:00]

Was there but accidentally removed
Done.

+      "source", app_runtime::ToString(app_runtime::LAUNCH_SOURCE_FILE_HANDLER));
+
+  base::ListValue* items = new base::ListValue();

[benwells, 2014/10/28 21:22:48]

Why is this all changing?

[cylee1, 2014/10/29 16:39:59]

replied above

   DCHECK(file_entries.size() == mime_types.size());
   for (size_t i = 0; i < file_entries.size(); ++i) {
-    scoped_ptr<base::DictionaryValue> launch_item(new base::DictionaryValue);
-    launch_item->SetString("fileSystemId", file_entries[i].filesystem_id);
-    launch_item->SetString("baseName", file_entries[i].registered_name);
-    launch_item->SetString("mimeType", mime_types[i]);
-    launch_item->SetString("entryId", file_entries[i].id);
-    items->Append(launch_item.release());
+    base::DictionaryValue* launch_item = new base::DictionaryValue();
+
+    base::DictionaryValue* raw_entry = new base::DictionaryValue();
+    raw_entry->SetString("fileSystemId", file_entries[i].filesystem_id);
+    raw_entry->SetString("baseName", file_entries[i].registered_name);
+    raw_entry->SetString("entryId", file_entries[i].id);
+
+    launch_item->Set("rawEntry", raw_entry);
+    launch_item->SetString("type", mime_types[i]);
+
+    items->Append(launch_item);
   }
-  launch_data->Set("items", items.release());
-  DispatchOnLaunchedEventImpl(extension->id(), launch_data.Pass(), context);
+  launch_data->Set("items", items);
+  DispatchOnLaunchedEventImpl(
+      extension->id(),
+      scoped_ptr<base::DictionaryValue>(launch_data).Pass(),
+      context);
 }
 
 // static
@@ -124,6 +177,9 @@ void AppRuntimeEventRouter::DispatchOnLaunchedEventWithUrl(
   launch_data.id.reset(new std::string(handler_id));
   launch_data.url.reset(new std::string(url.spec()));
   launch_data.referrer_url.reset(new std::string(referrer_url.spec()));
+  if (extensions::FeatureSwitch::trace_app_source()->IsEnabled()) {
+    launch_data.source = app_runtime::LAUNCH_SOURCE_URL_HANDLER;
+  }
   DispatchOnLaunchedEventImpl(
       extension->id(), launch_data.ToValue().Pass(), context);
 }