UMA stats for stylus usage

chrome/browser/chromeos/note_taking_app_utils.cc

 // Copyright 2016 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/chromeos/note_taking_app_utils.h"
 
 #include <string>
 #include <vector>
 
 #include "apps/launcher.h"
 #include "ash/common/system/chromeos/palette/palette_utils.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/memory/ptr_util.h"
 #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"
 #include "url/gurl.h"
 
 namespace app_runtime = extensions::api::app_runtime;
 
 namespace chromeos {
 namespace {
 
 // TODO(derat): Add more IDs.
 const char* const kExtensionIds[] = {
     // TODO(jdufault): Remove testing version after m54. See crbug.com/640828.
     "ogfjaccbdfhecploibfbhighmebiffla",  // Testing Keep app
     "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
   // designated by a to-be-added pref.
   const std::string switch_value =
       base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
           switches::kNoteTakingAppIds);
   if (!switch_value.empty()) {
     ids = base::SplitString(switch_value, ",", base::TRIM_WHITESPACE,
                             base::SPLIT_WANT_NONEMPTY);
   } 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);
     }
   }
   return nullptr;
 }
 
 }  // namespace
 
 bool IsNoteTakingAppAvailable(Profile* profile) {
   DCHECK(profile);
   return ash::IsPaletteFeatureEnabled() && GetApp(profile);
 }
 
 void LaunchNoteTakingAppForNewNote(Profile* profile,
                                    const base::FilePath& path) {
   DCHECK(profile);
   const extensions::Extension* app = GetApp(profile);
   if (!app) {
     LOG(ERROR) << "Failed to find note-taking app";
     return;
   }
 
   auto action_data = base::MakeUnique<app_runtime::ActionData>();
   action_data->action_type = app_runtime::ActionType::ACTION_TYPE_NEW_NOTE;
   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