Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(7849)

Unified Diff: chrome/browser/shell_integration_win.cc

Issue 2079233004: Add the Windows.IsPinnedToTaskbar metric. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Gab comments Created 4 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/shell_integration_win.cc
diff --git a/chrome/browser/shell_integration_win.cc b/chrome/browser/shell_integration_win.cc
index 87954bbf20853122cf2dbe7c68a0ef83659fe98c..81618c49d3a98eaed8f0e8de1d5a51842f15f3b5 100644
--- a/chrome/browser/shell_integration_win.cc
+++ b/chrome/browser/shell_integration_win.cc
@@ -43,6 +43,8 @@
#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"
@@ -55,6 +57,8 @@
#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;
@@ -421,6 +425,34 @@ class 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
Patrick Monette 2016/06/29 18:28:42 Done. The process error is still separate because
+
+ 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
Patrick Monette 2016/06/29 18:28:42 It needs to be 2 callbacks. In the general case, a
+
+ 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
Patrick Monette 2016/06/29 18:28:42 Done.
+}
+
} // namespace
bool SetAsDefaultBrowser() {
@@ -659,6 +691,25 @@ void MigrateTaskbarPins() {
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);

Powered by Google App Engine
This is Rietveld 408576698