Add the Windows.IsPinnedToTaskbar metric.

chrome/browser/shell_integration_win.cc

 // 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/shell_integration_win.h"
 
 #include <windows.h>
 #include <shlwapi.h>
 #include <shobjidl.h>
 #include <propkey.h>  // Needs to come after shobjidl.h.
@@ skipping 25 matching lines @@
 #include "base/win/scoped_propvariant.h"
 #include "base/win/shortcut.h"
 #include "base/win/windows_version.h"
 #include "chrome/browser/policy/policy_path_parser.h"
 #include "chrome/browser/shell_integration.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/browser/win/settings_app_monitor.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths_internal.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/shell_handler_win.mojom.h"
+#include "chrome/grit/generated_resources.h"
 #include "chrome/installer/setup/setup_util.h"
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/create_reg_key_work_item.h"
 #include "chrome/installer/util/install_util.h"
 #include "chrome/installer/util/scoped_user_protocol_entry.h"
 #include "chrome/installer/util/set_reg_value_work_item.h"
 #include "chrome/installer/util/shell_util.h"
 #include "chrome/installer/util/util_constants.h"
 #include "chrome/installer/util/work_item.h"
 #include "chrome/installer/util/work_item_list.h"
 #include "components/variations/variations_associated_data.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/utility_process_mojo_client.h"
+#include "ui/base/l10n/l10n_util.h"
 
 using content::BrowserThread;
 
 namespace shell_integration {
 
 namespace {
 
 // Helper function for GetAppId to generates profile id
 // from profile path. "profile_id" is composed of sanitized basenames of
 // user data dir and profile dir joined by a ".".
@@ skipping 346 matching lines @@
   // Weak ptrs are used to bind this class to the callbacks of the timer and the
   // registry watcher. This makes it possible to self-delete after one of the
   // callbacks is executed to cancel the remaining ones.
   base::WeakPtrFactory<OpenSystemSettingsHelper> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(OpenSystemSettingsHelper);
 };
 
 OpenSystemSettingsHelper* OpenSystemSettingsHelper::instance_ = nullptr;
 
+// Record the UMA histogram when a response is received. The callback that binds
+// to this function holds a reference to the ShellHandlerClient to keep it alive
+// until invokation.
+void OnIsPinnedToTaskbarResult(
+    content::UtilityProcessMojoClient<mojom::ShellHandler>* client,
+    bool succeeded,
+    bool is_pinned_to_taskbar) {
+  // Clean up the utility process.
+  delete client;
+
+  UMA_HISTOGRAM_BOOLEAN("Windows.IsPinnedToTaskbar.ProcessError", false);
+  UMA_HISTOGRAM_BOOLEAN("Windows.IsPinnedToTaskbar.Succeeded", succeeded);

[Alexei Svitkine (slow), 2016/06/29 14:37:44]

I don't see why you need multiple histograms. Just use a single histogram -
either logging true or false. Or you can make it an enum if you want more cases.

[Patrick Monette, 2016/06/29 18:28:42]

Done. The process error is still separate because it will move somewhere else
when more users of the ShellHandler Mojo interface are added.

+
+  if (succeeded)
+    UMA_HISTOGRAM_BOOLEAN("Windows.IsPinnedToTaskbar", is_pinned_to_taskbar);
+}
+
+// Called when a connection error happen with the shell handler process. A call
+// to this function is mutially exclusive with a call to
+// OnIsPinnedToTaskbarResult().
+void OnShellHandlerConnectionError(
+    content::UtilityProcessMojoClient<mojom::ShellHandler>* client) {
+  // Clean up the utility process.
+  delete client;

[Alexei Svitkine (slow), 2016/06/29 14:37:44]

Can the param be a scoped_ptr so you don't need to explicitly delete?

I guess it's not possible because of both success and error callback existing
and each needing to free it but only one will be called? That seems kind of
sub-optimal. Is it a limitation of Mojo?

Is there a way to use the same callback that owns the object with a
error/success param instead?

[Patrick Monette, 2016/06/29 18:28:42]

It needs to be 2 callbacks. In the general case, a Mojo interface can have
multiple functions with different parameters yet they share the same connection
error callback. 

+
+  UMA_HISTOGRAM_BOOLEAN("Windows.IsPinnedToTaskbar.ProcessError", true);

[Alexei Svitkine (slow), 2016/06/29 14:37:44]

A UMA macro expands to a lot of code. Please make a helper function for a
histogram you plan to log from multiple places, rather than repeating the macro.

[Patrick Monette, 2016/06/29 18:28:42]

Done.

+}
+
 }  // namespace
 
 bool SetAsDefaultBrowser() {
   base::FilePath chrome_exe;
   if (!PathService::Get(base::FILE_EXE, &chrome_exe)) {
     LOG(ERROR) << "Error getting app exe path";
     return false;
   }
 
   // From UI currently we only allow setting default browser for current user.
@@ skipping 218 matching lines @@
   // This needs to happen eventually (e.g. so that the appid is fixed and the
   // run-time Chrome icon is merged with the taskbar shortcut), but this is not
   // urgent and shouldn't delay Chrome startup.
   static const int64_t kMigrateTaskbarPinsDelaySeconds = 15;
   BrowserThread::PostDelayedTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&MigrateTaskbarPinsCallback),
       base::TimeDelta::FromSeconds(kMigrateTaskbarPinsDelaySeconds));
 }
 
+void RecordIsPinnedToTaskbarHistogram() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  // The code to check if Chrome is pinned to the taskbar brings in shell
+  // extensions which can hinder stability so it is executed in a utility
+  // process.
+  content::UtilityProcessMojoClient<mojom::ShellHandler>* client =
+      new content::UtilityProcessMojoClient<mojom::ShellHandler>(
+          l10n_util::GetStringUTF16(IDS_UTILITY_PROCESS_SHELL_HANDLER_NAME));
+
+  client->set_error_callback(
+      base::Bind(&OnShellHandlerConnectionError, client));
+  client->set_disable_sandbox();
+  client->Start();
+
+  client->service()->IsPinnedToTaskbar(
+      base::Bind(&OnIsPinnedToTaskbarResult, client));
+}
+
 int MigrateShortcutsInPathInternal(const base::FilePath& chrome_exe,
                                    const base::FilePath& path) {
   DCHECK(base::win::GetVersion() >= base::win::VERSION_WIN7);
 
   // Enumerate all pinned shortcuts in the given path directly.
   base::FileEnumerator shortcuts_enum(
       path, false,  // not recursive
       base::FileEnumerator::FILES, FILE_PATH_LITERAL("*.lnk"));
 
   bool is_per_user_install = InstallUtil::IsPerUserInstall(chrome_exe);
@@ skipping 119 matching lines @@
     if (base::PathExists(shortcut))
       return shortcut;
   }
 
   return base::FilePath();
 }
 
 }  // namespace win
 
 }  // namespace shell_integration