Add metrics to all items in the 'wrench' menu

chrome/browser/ui/toolbar/wrench_menu_model.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/ui/toolbar/wrench_menu_model.h"
 
 #include <algorithm>
 #include <cmath>
 
 #include "base/command_line.h"
+#include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/defaults.h"
 #include "chrome/browser/extensions/extension_toolbar_model.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/profiles/profile.h"
@@ skipping 248 matching lines @@
   AddCheckItemWithStringId(IDC_PROFILING_ENABLED, IDS_PROFILING_ENABLED);
 #endif
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // WrenchMenuModel
 
 WrenchMenuModel::WrenchMenuModel(ui::AcceleratorProvider* provider,
                                  Browser* browser)
     : ui::SimpleMenuModel(this),
+      uma_action_recorded_(false),
       provider_(provider),
       browser_(browser),
       tab_strip_model_(browser_->tab_strip_model()) {
   Build();
   UpdateZoomControls();
 
   content_zoom_subscription_ =
       content::HostZoomMap::GetDefaultForBrowserContext(browser->profile())
           ->AddZoomLevelChangedCallback(base::Bind(
               &WrenchMenuModel::OnZoomLevelChanged, base::Unretained(this)));
@@ skipping 108 matching lines @@
   if (!switches::IsNewAvatarMenu() && command_id == IDC_SHOW_SIGNIN) {
     // If a custom error message is being shown, handle it.
     GlobalError* error = signin_ui_util::GetSignedInServiceError(
         browser_->profile()->GetOriginalProfile());
     if (error) {
       error->ExecuteMenuItem(browser_);
       return;
     }
   }
 
-  if (command_id == IDC_HELP_PAGE_VIA_MENU)
-    content::RecordAction(UserMetricsAction("ShowHelpTabViaWrenchMenu"));
-
-  if (command_id == IDC_FULLSCREEN) {
-    // We issue the UMA command here and not in BrowserCommandController or even
-    // FullscreenController since we want to be able to distinguish this event
-    // and a menu which is under development.
-    content::RecordAction(UserMetricsAction("EnterFullScreenWithWrenchMenu"));
-  }
-
+  LogMenuMetrics(command_id);
   chrome::ExecuteCommand(browser_, command_id);
 }
 
+void WrenchMenuModel::LogMenuMetrics(int command_id) {
+  base::TimeDelta now = timer_.Elapsed();

[Alexei Svitkine (slow), 2014/11/17 18:51:18]

Nit: Use a different name - |now| implies it's a time, rather than a duration.
How about |duration| or |delta|?

[edwardjung, 2014/11/17 19:18:56]

Done.

+
+  switch (command_id) {
+    case IDC_NEW_TAB:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_NewTab", now);
+      LogMenuAction(MENU_ACTION_NEW_TAB);
+      break;
+    case IDC_NEW_WINDOW:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_NewWindow", now);
+      LogMenuAction(MENU_ACTION_NEW_WINDOW);
+      break;
+    case IDC_NEW_INCOGNITO_WINDOW:
+      if (!uma_action_recorded_)

[Alexei Svitkine (slow), 2014/11/17 18:51:18]

Nit: {}'s because body is multiline. Same for others below.

[edwardjung, 2014/11/17 19:18:56]

This is actually single line. We still want to record the menu action for usage
levels, just not the timing. Should I add a line break for readability?

[Alexei Svitkine (slow), 2014/11/17 19:57:04]

The style guide is not about number of statements, but number of lines (i.e.
including wrapping). So since the UMA statement spans multiple lines due to
wrapping, the if should have {}'s.

[edwardjung, 2014/11/17 23:45:26]

My bad, still getting used to the C++ style guide.

Corrected here and elsewhere.

+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_NewIncognitoWindow",
+                                   now);
+      LogMenuAction(MENU_ACTION_NEW_INCOGNITO_WINDOW);
+      break;
+
+    // Bookmarks sub menu.
+    case IDC_SHOW_BOOKMARK_BAR:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_ShowBookmarkBar",
+                                   now);
+      LogMenuAction(MENU_ACTION_SHOW_BOOKMARK_BAR);
+      break;
+    case IDC_SHOW_BOOKMARK_MANAGER:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_ShowBookmarkMgr",
+                                   now);
+      LogMenuAction(MENU_ACTION_SHOW_BOOKMARK_MANAGER);
+      break;
+    case IDC_IMPORT_SETTINGS:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_ImportSettings",
+                                   now);
+      LogMenuAction(MENU_ACTION_IMPORT_SETTINGS);
+      break;
+    case IDC_BOOKMARK_PAGE:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_BookmarkPage", now);
+      LogMenuAction(MENU_ACTION_BOOKMARK_PAGE);
+      break;
+    case IDC_BOOKMARK_ALL_TABS:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_BookmarkAllTabs",
+                                   now);
+      LogMenuAction(MENU_ACTION_BOOKMARK_ALL_TABS);
+      break;
+    case IDC_PIN_TO_START_SCREEN:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_PinToStartScreen",
+                                   now);
+      LogMenuAction(MENU_ACTION_PIN_TO_START_SCREEN);
+      break;
+
+    // Recent tabs menu.
+    case IDC_RESTORE_TAB:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_RestoreTab", now);
+      LogMenuAction(MENU_ACTION_RESTORE_TAB);
+      break;
+
+    // Windows.
+    case IDC_WIN_DESKTOP_RESTART:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_WinDesktopRestart",
+                                   now);
+      LogMenuAction(MENU_ACTION_WIN_DESKTOP_RESTART);
+      break;
+    case IDC_WIN8_METRO_RESTART:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_Win8MetroRestart",
+                                   now);
+      LogMenuAction(MENU_ACTION_WIN8_METRO_RESTART);
+      break;
+
+    case IDC_WIN_CHROMEOS_RESTART:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_ChromeOSRestart",
+                                   now);
+      LogMenuAction(MENU_ACTION_WIN_CHROMEOS_RESTART);
+      break;
+    case IDC_DISTILL_PAGE:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_DistillPage", now);
+      LogMenuAction(MENU_ACTION_DISTILL_PAGE);
+      break;
+    case IDC_SAVE_PAGE:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_SavePage", now);
+      LogMenuAction(MENU_ACTION_SAVE_PAGE);
+      break;
+    case IDC_FIND:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_Find", now);
+      LogMenuAction(MENU_ACTION_FIND);
+      break;
+    case IDC_PRINT:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_Print", now);
+      LogMenuAction(MENU_ACTION_PRINT);
+      break;
+
+    // Edit menu.
+    case IDC_CUT:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_Cut", now);
+      LogMenuAction(MENU_ACTION_CUT);
+      break;
+    case IDC_COPY:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_Copy", now);
+      LogMenuAction(MENU_ACTION_COPY);
+      break;
+    case IDC_PASTE:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_Paste", now);
+      LogMenuAction(MENU_ACTION_PASTE);
+      break;
+
+    // Tools menu.
+    case IDC_CREATE_HOSTED_APP:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_CreateHostedApp",
+                                   now);
+      LogMenuAction(MENU_ACTION_CREATE_HOSTED_APP);
+      break;
+    case IDC_CREATE_SHORTCUTS:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_CreateShortcuts",
+                                   now);
+      LogMenuAction(MENU_ACTION_CREATE_SHORTCUTS);
+      break;
+    case IDC_MANAGE_EXTENSIONS:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_ManageExtensions",
+                                   now);
+      LogMenuAction(MENU_ACTION_MANAGE_EXTENSIONS);
+      break;
+    case IDC_TASK_MANAGER:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_TaskManager", now);
+      LogMenuAction(MENU_ACTION_TASK_MANAGER);
+      break;
+    case IDC_CLEAR_BROWSING_DATA:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_ClearBrowsingData",
+                                   now);
+      LogMenuAction(MENU_ACTION_CLEAR_BROWSING_DATA);
+      break;
+    case IDC_VIEW_SOURCE:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_ViewSource", now);
+      LogMenuAction(MENU_ACTION_VIEW_SOURCE);
+      break;
+    case IDC_DEV_TOOLS:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_DevTools", now);
+      LogMenuAction(MENU_ACTION_DEV_TOOLS);
+      break;
+    case IDC_DEV_TOOLS_CONSOLE:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_DevToolsConsole",
+                                   now);
+      LogMenuAction(MENU_ACTION_DEV_TOOLS_CONSOLE);
+      break;
+    case IDC_DEV_TOOLS_DEVICES:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_DevToolsDevices",
+                                   now);
+      LogMenuAction(MENU_ACTION_DEV_TOOLS_DEVICES);
+      break;
+    case IDC_PROFILING_ENABLED:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_ProfilingEnabled",
+                                   now);
+      LogMenuAction(MENU_ACTION_PROFILING_ENABLED);
+      break;
+
+    // Zoom menu
+    case IDC_ZOOM_MINUS:
+      if (!uma_action_recorded_) {
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_ZoomMinus", now);
+        LogMenuAction(MENU_ACTION_ZOOM_MINUS);
+      }
+      break;
+    case IDC_ZOOM_PLUS:
+      if (!uma_action_recorded_) {
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_ZoomPlus", now);
+        LogMenuAction(MENU_ACTION_ZOOM_PLUS);
+      }
+      break;
+    case IDC_FULLSCREEN:
+      content::RecordAction(UserMetricsAction("EnterFullScreenWithWrenchMenu"));
+
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_EnterFullScreen",
+                                   now);
+      LogMenuAction(MENU_ACTION_FULLSCREEN);
+      break;
+
+    case IDC_SHOW_HISTORY:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_ShowHistory", now);
+      LogMenuAction(MENU_ACTION_SHOW_HISTORY);
+      break;
+    case IDC_SHOW_DOWNLOADS:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_ShowDownloads",
+                                   now);
+      LogMenuAction(MENU_ACTION_SHOW_DOWNLOADS);
+      break;
+    case IDC_SHOW_SYNC_SETUP:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_ShowSyncSetup",
+                                   now);
+      LogMenuAction(MENU_ACTION_SHOW_SYNC_SETUP);
+      break;
+    case IDC_OPTIONS:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_Settings", now);
+      LogMenuAction(MENU_ACTION_OPTIONS);
+      break;
+    case IDC_ABOUT:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_About", now);
+      LogMenuAction(MENU_ACTION_ABOUT);
+      break;
+
+    // Help menu.
+    case IDC_HELP_PAGE_VIA_MENU:
+      content::RecordAction(UserMetricsAction("ShowHelpTabViaWrenchMenu"));
+
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_HelpPage", now);
+      LogMenuAction(MENU_ACTION_HELP_PAGE_VIA_MENU);
+      break;
+  #if defined(GOOGLE_CHROME_BUILD)
+    case IDC_FEEDBACK:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_Feedback", now);
+      LogMenuAction(MENU_ACTION_FEEDBACK);
+      break;
+  #endif
+
+    case IDC_TOGGLE_REQUEST_TABLET_SITE:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_RequestTabletSite",
+                                   now);
+      LogMenuAction(MENU_ACTION_TOGGLE_REQUEST_TABLET_SITE);
+      break;
+    case IDC_EXIT:
+      if (!uma_action_recorded_)
+        UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction_Exit", now);
+      LogMenuAction(MENU_ACTION_EXIT);
+      break;
+  }
+
+  if (!uma_action_recorded_) {
+    UMA_HISTOGRAM_MEDIUM_TIMES("WrenchMenu.TimeToAction", now);
+    uma_action_recorded_ = true;
+  }
+}
+
+void WrenchMenuModel::LogMenuAction(int action_id) {
+  UMA_HISTOGRAM_ENUMERATION("WrenchMenu.MenuAction", action_id,
+                                LIMIT_MENU_ACTION);

[Alexei Svitkine (slow), 2014/11/17 18:51:18]

Nit: Align.

[edwardjung, 2014/11/17 19:18:56]

Done.

+}
+
 bool WrenchMenuModel::IsCommandIdChecked(int command_id) const {
   if (command_id == IDC_SHOW_BOOKMARK_BAR) {
     return browser_->profile()->GetPrefs()->GetBoolean(
         bookmarks::prefs::kShowBookmarkBar);
   } else if (command_id == IDC_PROFILING_ENABLED) {
     return Profiling::BeingProfiled();
   } else if (command_id == IDC_TOGGLE_REQUEST_TABLET_SITE) {
     return chrome::IsRequestingTabletSite(browser_);
   }
 
@@ skipping 228 matching lines @@
   if (browser_->host_desktop_type() == chrome::HOST_DESKTOP_TYPE_ASH)
     show_exit_menu = false;
 #endif
 
   if (show_exit_menu) {
     AddSeparator(ui::NORMAL_SEPARATOR);
     AddItemWithStringId(IDC_EXIT, IDS_EXIT);
   }
 
   RemoveTrailingSeparators();
+  uma_action_recorded_ = false;
 }
 
 void WrenchMenuModel::AddGlobalErrorMenuItems() {
   // TODO(sail): Currently we only build the wrench menu once per browser
   // window. This means that if a new error is added after the menu is built
   // it won't show in the existing wrench menu. To fix this we need to some
   // how update the menu if new errors are added.
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
   // GetSignedInServiceErrors() can modify the global error list, so call it
   // before iterating through that list below.
@@ skipping 105 matching lines @@
                        ->GetZoomPercent();
   }
   zoom_label_ = l10n_util::GetStringFUTF16(
       IDS_ZOOM_PERCENT, base::IntToString16(zoom_percent));
 }
 
 void WrenchMenuModel::OnZoomLevelChanged(
     const content::HostZoomMap::ZoomLevelChange& change) {
   UpdateZoomControls();
 }