UMA stats for stylus usage

chrome/browser/chromeos/note_taking_app_utils.cc

Index: chrome/browser/chromeos/note_taking_app_utils.cc
diff --git a/chrome/browser/chromeos/note_taking_app_utils.cc b/chrome/browser/chromeos/note_taking_app_utils.cc
index 7f12a8da4081447fc6d397c1fde54acc820fe55b..106b2b659c98f00fb93d3d5af9e6c2078fd1f9d1 100644
--- a/chrome/browser/chromeos/note_taking_app_utils.cc
+++ b/chrome/browser/chromeos/note_taking_app_utils.cc
@@ -15,6 +15,7 @@
 #include "base/strings/string_split.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chromeos/chromeos_switches.h"
+#include "extensions/browser/app_window/app_window.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/api/app_runtime.h"
 #include "extensions/common/extension.h"
@@ -32,12 +33,8 @@ const char* const kExtensionIds[] = {
     "hmjkmjkepdijhoojdojkdfohbdgmmhki",  // Google Keep app (Web Store)
 };
 
-// Returns the first installed and enabled whitelisted note-taking app, or null
-// if none is installed.
-const extensions::Extension* GetApp(Profile* profile) {
-  // TODO(derat): Check the to-be-added "note-taking app enabled" pref here and
-  // return null if it's disabled.
-
+// Returns a list of app IDs.
+std::vector<std::string> GetAppIds() {
   std::vector<std::string> ids;
 
   // TODO(derat): Instead of a using hardcoded list of IDs, use an app
@@ -51,12 +48,20 @@ const extensions::Extension* GetApp(Profile* profile) {
   } else {
     ids.assign(kExtensionIds, kExtensionIds + arraysize(kExtensionIds));
   }
+  return ids;
+}
+
+// Returns the first installed and enabled whitelisted note-taking app, or null
+// if none is installed.
+const extensions::Extension* GetApp(Profile* profile) {
+  // TODO(derat): Check the to-be-added "note-taking app enabled" pref here and
+  // return null if it's disabled.
 
   const extensions::ExtensionRegistry* extension_registry =
       extensions::ExtensionRegistry::Get(profile);
   const extensions::ExtensionSet& enabled_extensions =
       extension_registry->enabled_extensions();
-  for (const auto& id : ids) {
+  for (const auto& id : GetAppIds()) {
     if (enabled_extensions.Contains(id)) {
       return extension_registry->GetExtensionById(
           id, extensions::ExtensionRegistry::ENABLED);
@@ -86,4 +91,12 @@ void LaunchNoteTakingAppForNewNote(Profile* profile,
   apps::LaunchPlatformAppWithAction(profile, app, std::move(action_data), path);
 }
 
+bool IsNoteTakingApp(extensions::AppWindow* app_window) {
+  for (const auto& id : GetAppIds()) {

[Daniel Erat, 2016/09/19 22:08:47]

the changelist description describes us as measuring events in the "default
note-taking app", but this looks like it's checking against all of the hardcoded
IDs instead of just the one that we'd launch via
LaunchNoteTakingAppForNewNote(). is that intentional? if not, it seems like it
may be more appropriate to call GetApp() here and compare its ID (if it's
non-null) against the window's ID. in particular, once we expose a pref to
choose a different app, we'd probably only want to compare against that app
here, right?

(this all comes down to how this metric is going to be used, though, and the
answer to that isn't clear to me from looking at the bug.)

[xiaoyinh(OOO Sep 11-29), 2016/09/19 22:45:49]

Thank you for your comments. GetApp will return the first installed and enabled
app, does that means we will have only one default note taking app in the
system? I was a little confused with the list of extension ids and thought they
may belong to different versions of the extension.

+    if (id == app_window->extension_id())
+      return true;
+  }
+  return false;
+}
+
 }  // namespace chromeos