Commit 6b9706d9 authored by Karan Bhatia's avatar Karan Bhatia Committed by Commit Bot

Click to script: Record metrics for action chosen through extension icon context menu.

This CL adds metrics for the action taken by the user through the extension icon
context menu.

BUG=885201

Change-Id: I5eb2261f493298667ec978d24e554b15814e5f84
Reviewed-on: https://chromium-review.googlesource.com/1239217
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarIlya Sherman <isherman@chromium.org>
Reviewed-by: default avatarIstiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593782}
parent fdeeb64b
......@@ -5,6 +5,7 @@
#include "chrome/browser/extensions/extension_context_menu_model.h"
#include "base/macros.h"
#include "base/metrics/histogram_macros.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/extensions/chrome_extension_browser_constants.h"
......@@ -105,6 +106,38 @@ bool IsExtensionRequiredByPolicy(const Extension* extension,
policy->MustRemainInstalled(extension, nullptr);
}
ExtensionContextMenuModel::ContextMenuAction CommandIdToContextMenuAction(
int command_id) {
using ContextMenuAction = ExtensionContextMenuModel::ContextMenuAction;
switch (command_id) {
case ExtensionContextMenuModel::HOME_PAGE:
return ContextMenuAction::kHomePage;
case ExtensionContextMenuModel::OPTIONS:
return ContextMenuAction::kOptions;
case ExtensionContextMenuModel::TOGGLE_VISIBILITY:
return ContextMenuAction::kToggleVisibility;
case ExtensionContextMenuModel::UNINSTALL:
return ContextMenuAction::kUninstall;
case ExtensionContextMenuModel::MANAGE_EXTENSIONS:
return ContextMenuAction::kManageExtensions;
case ExtensionContextMenuModel::INSPECT_POPUP:
return ContextMenuAction::kInspectPopup;
case ExtensionContextMenuModel::PAGE_ACCESS_RUN_ON_CLICK:
return ContextMenuAction::kPageAccessRunOnClick;
case ExtensionContextMenuModel::PAGE_ACCESS_RUN_ON_SITE:
return ContextMenuAction::kPageAccessRunOnSite;
case ExtensionContextMenuModel::PAGE_ACCESS_RUN_ON_ALL_SITES:
return ContextMenuAction::kPageAccessRunOnAllSites;
case ExtensionContextMenuModel::PAGE_ACCESS_LEARN_MORE:
return ContextMenuAction::kPageAccessLearnMore;
default:
break;
}
NOTREACHED();
return ContextMenuAction::kNoAction;
}
// A stub for the uninstall dialog.
// TODO(devlin): Ideally, we would just have the uninstall dialog take a
// base::Callback, but that's a bunch of churn.
......@@ -241,9 +274,12 @@ void ExtensionContextMenuModel::ExecuteCommand(int command_id,
DCHECK(extension_items_);
extension_items_->ExecuteCommand(command_id, GetActiveWebContents(),
nullptr, content::ContextMenuParams());
action_taken_ = ContextMenuAction::kCustomCommand;
return;
}
action_taken_ = CommandIdToContextMenuAction(command_id);
switch (command_id) {
case HOME_PAGE: {
content::OpenURLParams params(ManifestURL::GetHomepageURL(extension),
......@@ -287,6 +323,18 @@ void ExtensionContextMenuModel::ExecuteCommand(int command_id,
}
}
void ExtensionContextMenuModel::OnMenuWillShow(ui::SimpleMenuModel* menu) {
action_taken_ = ContextMenuAction::kNoAction;
}
void ExtensionContextMenuModel::MenuClosed(ui::SimpleMenuModel* menu) {
if (action_taken_) {
ContextMenuAction action = *action_taken_;
UMA_HISTOGRAM_ENUMERATION("Extensions.ContextMenuAction", action);
action_taken_ = base::nullopt;
}
}
ExtensionContextMenuModel::~ExtensionContextMenuModel() {}
void ExtensionContextMenuModel::InitMenu(const Extension* extension,
......
......@@ -9,6 +9,7 @@
#include <string>
#include "base/macros.h"
#include "base/optional.h"
#include "ui/base/models/simple_menu_model.h"
class Browser;
......@@ -39,6 +40,31 @@ class ExtensionContextMenuModel : public ui::SimpleMenuModel,
PAGE_ACCESS_RUN_ON_SITE,
PAGE_ACCESS_RUN_ON_ALL_SITES,
PAGE_ACCESS_LEARN_MORE,
// NOTE: If you update this, you probably need to update the
// ContextMenuAction enum below.
};
// A separate enum to indicate the action taken on the menu. We have two
// enums (this and MenuEntries above) to avoid needing to have a single one
// with both action-specific values (like kNoAction) and menu-specific values
// (like PAGE_ACCESS_SUBMENU).
// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused. New values should be added before
// kMaxValue.
enum class ContextMenuAction {
kNoAction = 0,
kCustomCommand = 1,
kHomePage = 2,
kOptions = 3,
kToggleVisibility = 4,
kUninstall = 5,
kManageExtensions = 6,
kInspectPopup = 7,
kPageAccessRunOnClick = 8,
kPageAccessRunOnSite = 9,
kPageAccessRunOnAllSites = 10,
kPageAccessLearnMore = 11,
kMaxValue = kPageAccessLearnMore,
};
// Type of action the extension icon represents.
......@@ -82,6 +108,8 @@ class ExtensionContextMenuModel : public ui::SimpleMenuModel,
bool IsCommandIdVisible(int command_id) const override;
bool IsCommandIdEnabled(int command_id) const override;
void ExecuteCommand(int command_id, int event_flags) override;
void OnMenuWillShow(ui::SimpleMenuModel* source) override;
void MenuClosed(ui::SimpleMenuModel* source) override;
ui::SimpleMenuModel* page_access_submenu_for_testing() {
return page_access_submenu_.get();
......@@ -136,6 +164,10 @@ class ExtensionContextMenuModel : public ui::SimpleMenuModel,
std::unique_ptr<ui::SimpleMenuModel> page_access_submenu_;
// The action taken by the menu. Has a valid value when the menu is being
// shown.
base::Optional<ContextMenuAction> action_taken_;
DISALLOW_COPY_AND_ASSIGN(ExtensionContextMenuModel);
};
......
......@@ -9,6 +9,7 @@
#include "base/macros.h"
#include "base/memory/ptr_util.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
......@@ -1309,4 +1310,88 @@ TEST_F(ExtensionContextMenuModelTest, TestClickingPageAccessLearnMore) {
web_contents->GetLastCommittedURL());
}
TEST_F(ExtensionContextMenuModelTest, HistogramTest_Basic) {
InitializeEmptyExtensionService();
scoped_refptr<const Extension> extension =
ExtensionBuilder("extension").Build();
InitializeAndAddExtension(*extension);
constexpr char kHistogramName[] = "Extensions.ContextMenuAction";
{
base::HistogramTester tester;
{
// The menu is constructed, but never shown.
ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
ExtensionContextMenuModel::VISIBLE,
nullptr);
}
tester.ExpectTotalCount(kHistogramName, 0);
}
{
base::HistogramTester tester;
{
// The menu is constructed and shown, but no action is taken.
ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
ExtensionContextMenuModel::VISIBLE,
nullptr);
menu.OnMenuWillShow(&menu);
menu.MenuClosed(&menu);
}
tester.ExpectUniqueSample(
kHistogramName, ExtensionContextMenuModel::ContextMenuAction::kNoAction,
1 /* expected_count */);
}
{
base::HistogramTester tester;
{
// The menu is constructed, shown, and an action taken.
ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
ExtensionContextMenuModel::VISIBLE,
nullptr);
menu.OnMenuWillShow(&menu);
menu.ExecuteCommand(ExtensionContextMenuModel::MANAGE_EXTENSIONS, 0);
menu.MenuClosed(&menu);
}
tester.ExpectUniqueSample(
kHistogramName,
ExtensionContextMenuModel::ContextMenuAction::kManageExtensions,
1 /* expected_count */);
}
}
TEST_F(ExtensionContextMenuModelTest, HistogramTest_CustomCommand) {
constexpr char kHistogramName[] = "Extensions.ContextMenuAction";
InitializeEmptyExtensionService();
scoped_refptr<const Extension> extension =
ExtensionBuilder("extension")
.SetAction(ExtensionBuilder::ActionType::BROWSER_ACTION)
.Build();
InitializeAndAddExtension(*extension);
// Create a MenuManager for adding context items.
MenuManager* manager = static_cast<MenuManager*>(
(MenuManagerFactory::GetInstance()->SetTestingFactoryAndUse(
profile(), &MenuManagerFactory::BuildServiceInstanceForTesting)));
ASSERT_TRUE(manager);
MenuBuilder builder(extension, GetBrowser(), manager);
builder.AddContextItem(MenuItem::BROWSER_ACTION);
std::unique_ptr<ExtensionContextMenuModel> menu = builder.BuildMenu();
EXPECT_EQ(1, CountExtensionItems(*menu));
base::HistogramTester tester;
menu->OnMenuWillShow(menu.get());
menu->ExecuteCommand(
ContextMenuMatcher::ConvertToExtensionsCustomCommandId(0), 0);
menu->MenuClosed(menu.get());
tester.ExpectUniqueSample(
kHistogramName,
ExtensionContextMenuModel::ContextMenuAction::kCustomCommand,
1 /* expected_count */);
}
} // namespace extensions
......@@ -15026,6 +15026,21 @@ Called by update_net_error_codes.py.-->
<int value="3" label="Dismiss by deactivation"/>
</enum>
<enum name="ExtensionContextMenuAction">
<int value="0" label="kNoAction"/>
<int value="1" label="kCustomCommand"/>
<int value="2" label="kHomePage"/>
<int value="3" label="kOptions"/>
<int value="4" label="kToggleVisibility"/>
<int value="5" label="kUninstall"/>
<int value="6" label="kManageExtensions"/>
<int value="7" label="kInspectPopup"/>
<int value="8" label="kPageAccessRunOnClick"/>
<int value="9" label="kPageAccessRunOnSite"/>
<int value="10" label="kPageAccessRunOnAllSites"/>
<int value="11" label="kPageAccessLearnMore"/>
</enum>
<enum name="ExtensionCreationFlags">
<int value="0" label="REQUIRE_KEY"/>
<int value="1" label="REQUIRE_MODERN_MANIFEST_VERSION"/>
......@@ -28726,6 +28726,15 @@ uploading your change for review.
</summary>
</histogram>
<histogram name="Extensions.ContextMenuAction"
enum="ExtensionContextMenuAction" expires_after="2019-12-30">
<owner>rdevlin.cronin@chromium.org</owner>
<owner>karandeepb@chromium.org</owner>
<summary>
Records the action taken by the user from the extension icon context menu.
</summary>
</histogram>
<histogram name="Extensions.CorruptExtensionBecameDisabled">
<owner>asargent@chromium.org</owner>
<summary>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment