Skip to content

Commit

Permalink
multipaste: Add metric for plaintext pastes
Browse files Browse the repository at this point in the history 
This CL adds a histogram tracking the number of times users select items
from the clipboard history menu while holding the shift key, triggering
plaintext pastes. This metric can be used in conjunction with
Ash.ClipboardHistory.Operation to estimate the percentage of pastes that
users want to appear in plaintext.

Fixed: 1133964
Change-Id: I2dd38e102603c695a650b2d25e0f366714975970
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3623884
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Commit-Queue: Colin Kincaid <ckincaid@chromium.org>
Reviewed-by: David Black <dmblack@google.com>
Cr-Commit-Position: refs/heads/main@{#1001217}
  • Loading branch information
Colin Kincaid authored and Chromium LUCI CQ committed May 9, 2022
1 parent e591d31 commit e0c3f02
Show file tree
Hide file tree
Showing 5 changed files with 213 additions and 23 deletions.
62 changes: 54 additions & 8 deletions ash/clipboard/clipboard_history_controller_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,25 @@ void EncodeBitmapToPNG(
std::move(barrier_callback).Run();
}

using ClipboardHistoryPasteType =
ash::ClipboardHistoryControllerImpl::ClipboardHistoryPasteType;
bool IsPlainTextPaste(ClipboardHistoryPasteType paste_type) {
switch (paste_type) {
case ClipboardHistoryPasteType::kPlainTextAccelerator:
case ClipboardHistoryPasteType::kPlainTextKeystroke:
case ClipboardHistoryPasteType::kPlainTextMouse:
case ClipboardHistoryPasteType::kPlainTextTouch:
case ClipboardHistoryPasteType::kPlainTextVirtualKeyboard:
return true;
case ClipboardHistoryPasteType::kRichTextAccelerator:
case ClipboardHistoryPasteType::kRichTextKeystroke:
case ClipboardHistoryPasteType::kRichTextMouse:
case ClipboardHistoryPasteType::kRichTextTouch:
case ClipboardHistoryPasteType::kRichTextVirtualKeyboard:
return false;
}
}

} // namespace

// ClipboardHistoryControllerImpl::AcceleratorTarget ---------------------------
Expand Down Expand Up @@ -256,7 +275,7 @@ void ClipboardHistoryControllerImpl::ToggleMenuShownByAccelerator() {
// if none is selected.
PasteMenuItemData(context_menu_->GetSelectedMenuItemCommand().value_or(
ClipboardHistoryUtil::kFirstItemCommandId),
/*paste_plain_text=*/false);
ClipboardHistoryPasteType::kRichTextAccelerator);
return;
}

Expand Down Expand Up @@ -557,7 +576,7 @@ bool ClipboardHistoryControllerImpl::PasteClipboardItemById(
base::BindOnce(
&ClipboardHistoryControllerImpl::PasteClipboardHistoryItem,
weak_ptr_factory_.GetWeakPtr(), active_window, item,
/*paste_plain_text=*/false));
ClipboardHistoryPasteType::kRichTextVirtualKeyboard));
return true;
}
}
Expand Down Expand Up @@ -651,8 +670,31 @@ void ClipboardHistoryControllerImpl::ExecuteCommand(int command_id,
Action action = context_menu_->GetActionForCommandId(command_id);
switch (action) {
case Action::kPaste:
PasteMenuItemData(command_id, event_flags & ui::EF_SHIFT_DOWN);
return;
// Create a scope for the variables used in this case so that they can be
// deallocated from the stack.
{
bool paste_plain_text = event_flags & ui::EF_SHIFT_DOWN;
// There are no specific flags that indicate a paste triggered by a
// keystroke, so assume by default that keystroke was the event source
// and then check for the other known possibilities. This assumption may
// cause pastes from unknown sources to be incorrectly captured as
// keystroke pastes, but we do not expect such cases to significantly
// alter metrics.
ClipboardHistoryPasteType paste_type =
paste_plain_text ? ClipboardHistoryPasteType::kPlainTextKeystroke
: ClipboardHistoryPasteType::kRichTextKeystroke;
if (event_flags & ui::EF_MOUSE_BUTTON) {
paste_type = paste_plain_text
? ClipboardHistoryPasteType::kPlainTextMouse
: ClipboardHistoryPasteType::kRichTextMouse;
} else if (event_flags & ui::EF_FROM_TOUCH) {
paste_type = paste_plain_text
? ClipboardHistoryPasteType::kPlainTextTouch
: ClipboardHistoryPasteType::kRichTextTouch;
}
PasteMenuItemData(command_id, paste_type);
return;
}
case Action::kDelete:
DeleteItemWithCommandId(command_id);
return;
Expand All @@ -668,8 +710,9 @@ void ClipboardHistoryControllerImpl::ExecuteCommand(int command_id,
}
}

void ClipboardHistoryControllerImpl::PasteMenuItemData(int command_id,
bool paste_plain_text) {
void ClipboardHistoryControllerImpl::PasteMenuItemData(
int command_id,
ClipboardHistoryPasteType paste_type) {
UMA_HISTOGRAM_ENUMERATION(
"Ash.ClipboardHistory.ContextMenu.MenuOptionSelected", command_id,
ClipboardHistoryUtil::kMaxCommandId);
Expand All @@ -694,13 +737,13 @@ void ClipboardHistoryControllerImpl::PasteMenuItemData(int command_id,
FROM_HERE,
base::BindOnce(&ClipboardHistoryControllerImpl::PasteClipboardHistoryItem,
weak_ptr_factory_.GetWeakPtr(), active_window,
selected_item, paste_plain_text));
selected_item, paste_type));
}

void ClipboardHistoryControllerImpl::PasteClipboardHistoryItem(
aura::Window* intended_window,
ClipboardHistoryItem item,
bool paste_plain_text) {
ClipboardHistoryPasteType paste_type) {
// It's possible that the window could change or we could enter a disabled
// mode after posting the `PasteClipboardHistoryItem()` task.
if (!intended_window || intended_window != window_util::GetActiveWindow() ||
Expand All @@ -717,6 +760,7 @@ void ClipboardHistoryControllerImpl::PasteClipboardHistoryItem(
// we can paste the selected history item.
ui::DataTransferEndpoint data_dst(ui::EndpointType::kClipboardHistory);
const auto* current_clipboard_data = clipboard->GetClipboardData(&data_dst);
bool paste_plain_text = IsPlainTextPaste(paste_type);
if (paste_plain_text || !current_clipboard_data ||
*current_clipboard_data != item.data()) {
std::unique_ptr<ui::ClipboardData> temp_data;
Expand Down Expand Up @@ -757,6 +801,8 @@ void ClipboardHistoryControllerImpl::PasteClipboardHistoryItem(

++pastes_to_be_confirmed_;

base::UmaHistogramEnumeration("Ash.ClipboardHistory.PasteType", paste_type);

for (auto& observer : observers_)
observer.OnClipboardHistoryPasted();

Expand Down
30 changes: 24 additions & 6 deletions ash/clipboard/clipboard_history_controller_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,23 @@ class ASH_EXPORT ClipboardHistoryControllerImpl
public ClipboardHistory::Observer,
public ClipboardHistoryResourceManager::Observer {
public:
// Source and plain vs. rich text info for each paste. These values are used
// in the Ash.ClipboardHistory.PasteType histogram and therefore cannot be
// reordered. New types may be appended to the end of this enumeration.
enum class ClipboardHistoryPasteType {
kPlainTextAccelerator = 0, // Plain text paste triggered by accelerator
kRichTextAccelerator = 1, // Rich text paste triggered by accelerator
kPlainTextKeystroke = 2, // Plain text paste triggered by keystroke
kRichTextKeystroke = 3, // Rich text paste triggered by keystroke
kPlainTextMouse = 4, // Plain text paste triggered by mouse click
kRichTextMouse = 5, // Rich text paste triggered by mouse click
kPlainTextTouch = 6, // Plain text paste triggered by gesture tap
kRichTextTouch = 7, // Rich text paste triggered by gesture tap
kPlainTextVirtualKeyboard = 8, // Plain text paste triggered by VK request
kRichTextVirtualKeyboard = 9, // Rich text paste triggered by VK request
kMaxValue = 9
};

ClipboardHistoryControllerImpl();
ClipboardHistoryControllerImpl(const ClipboardHistoryControllerImpl&) =
delete;
Expand Down Expand Up @@ -165,19 +182,20 @@ class ASH_EXPORT ClipboardHistoryControllerImpl
std::unique_ptr<std::map<base::UnguessableToken, std::vector<uint8_t>>>
encoded_pngs);

// Executes the command specified by `command_id` with the given event flags.
// Executes the command specified by `command_id` with the given
// `event_flags`.
void ExecuteCommand(int command_id, int event_flags);

// Paste the clipboard data of the menu item specified by `command_id`.
// `paste_plain_text` indicates whether the plain text instead of the
// clipboard data should be pasted.
void PasteMenuItemData(int command_id, bool paste_plain_text);
void PasteMenuItemData(int command_id, ClipboardHistoryPasteType paste_type);

// Pastes the specified clipboard history item, if |intended_window| matches
// the active window.
// the active window. `paste_type` indicates the source of the paste for
// metrics tracking as well as whether plain text should be pasted instead of
// the full, rich-text clipboard data.
void PasteClipboardHistoryItem(aura::Window* intended_window,
ClipboardHistoryItem item,
bool paste_plain_text);
ClipboardHistoryPasteType paste_type);

// Delete the menu item being selected and its corresponding data. If no item
// is selected, do nothing.
Expand Down
121 changes: 112 additions & 9 deletions chrome/browser/ui/ash/clipboard_history_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,9 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryBrowserTest, DeleteItemViaBackspaceKey) {

IN_PROC_BROWSER_TEST_F(ClipboardHistoryBrowserTest,
ShouldPasteHistoryAsPlainText) {
using ClipboardHistoryPasteType =
ash::ClipboardHistoryControllerImpl::ClipboardHistoryPasteType;

// Increase delay interval before restoring the clipboard buffer following
// a paste event as this test has exhibited flakiness due to the amount of
// time it takes a paste event to reach the web contents under test. Remove
Expand Down Expand Up @@ -740,6 +743,9 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryBrowserTest,
};

// Confirm initial state.
base::HistogramTester histogram_tester;
histogram_tester.ExpectTotalCount("Ash.ClipboardHistory.PasteType",
/*count=*/0);
ASSERT_TRUE(GetLastPaste().empty());

// Write some things to the clipboard.
Expand Down Expand Up @@ -771,12 +777,49 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryBrowserTest,
ASSERT_EQ(last_paste.size(), 2u);
EXPECT_EQ(last_paste[0].GetString(), "text/plain: A");
EXPECT_EQ(last_paste[1].GetString(), "text/html: <span>A</span>");
histogram_tester.ExpectBucketCount(
"Ash.ClipboardHistory.PasteType",
ClipboardHistoryPasteType::kRichTextKeystroke,
/*expected_count=*/1);
histogram_tester.ExpectTotalCount("Ash.ClipboardHistory.PasteType",
/*count=*/1);

// Wait for the clipboard buffer to be restored before performing another
// paste. In production, this should happen faster than a user is able to
// relaunch clipboard history UI (knock on wood).
ClipboardDataWaiter().WaitFor(&clipboard_data);

// Open clipboard history and paste the last history item while holding down
// a non-shift key (arbitrarily, the alt key). The item should not paste as
// plain text.
ShowContextMenuViaAccelerator(/*wait_for_selection=*/true);
EXPECT_TRUE(GetClipboardHistoryController()->IsMenuShowing());
PressAndRelease(ui::KeyboardCode::VKEY_DOWN);
PressAndRelease(ui::KeyboardCode::VKEY_DOWN);
PressAndRelease(ui::KeyboardCode::VKEY_RETURN, ui::EF_ALT_DOWN);
EXPECT_FALSE(GetClipboardHistoryController()->IsMenuShowing());

// Wait for the paste event to propagate to the web contents.
// The web contents will notify us a paste occurred by updating page title.
std::ignore =
content::TitleWatcher(web_contents, u"Paste 2").WaitAndGetTitle();

// Confirm the expected paste data.
last_paste = GetLastPaste();
ASSERT_EQ(last_paste.size(), 2u);
EXPECT_EQ(last_paste[0].GetString(), "text/plain: A");
EXPECT_EQ(last_paste[1].GetString(), "text/html: <span>A</span>");
histogram_tester.ExpectBucketCount(
"Ash.ClipboardHistory.PasteType",
ClipboardHistoryPasteType::kRichTextKeystroke,
/*expected_count=*/2);
histogram_tester.ExpectTotalCount("Ash.ClipboardHistory.PasteType",
/*count=*/2);

// Wait for the clipboard buffer to be restored before performing another
// paste.
ClipboardDataWaiter().WaitFor(&clipboard_data);

// Open clipboard history and paste the last history item while holding down
// the shift key. The item should paste as plain text.
ShowContextMenuViaAccelerator(/*wait_for_selection=*/true);
Expand All @@ -789,12 +832,18 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryBrowserTest,
// Wait for the paste event to propagate to the web contents.
// The web contents will notify us a paste occurred by updating page title.
std::ignore =
content::TitleWatcher(web_contents, u"Paste 2").WaitAndGetTitle();
content::TitleWatcher(web_contents, u"Paste 3").WaitAndGetTitle();

// Confirm the expected paste data.
last_paste = GetLastPaste();
ASSERT_EQ(last_paste.size(), 1u);
EXPECT_EQ(last_paste[0].GetString(), "text/plain: A");
histogram_tester.ExpectBucketCount(
"Ash.ClipboardHistory.PasteType",
ClipboardHistoryPasteType::kPlainTextKeystroke,
/*expected_count=*/1);
histogram_tester.ExpectTotalCount("Ash.ClipboardHistory.PasteType",
/*count=*/3);

// Wait for the clipboard buffer to be restored before performing another
// paste.
Expand All @@ -812,38 +861,83 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryBrowserTest,
// Wait for the paste event to propagate to the web contents.
// The web contents will notify us a paste occurred by updating page title.
std::ignore =
content::TitleWatcher(web_contents, u"Paste 3").WaitAndGetTitle();
content::TitleWatcher(web_contents, u"Paste 4").WaitAndGetTitle();

// Confirm the expected paste data.
last_paste = GetLastPaste();
ASSERT_EQ(last_paste.size(), 2u);
EXPECT_EQ(last_paste[0].GetString(), "text/plain: A");
EXPECT_EQ(last_paste[1].GetString(), "text/html: <span>A</span>");
histogram_tester.ExpectBucketCount(
"Ash.ClipboardHistory.PasteType",
ClipboardHistoryPasteType::kRichTextAccelerator,
/*expected_count=*/1);
histogram_tester.ExpectTotalCount("Ash.ClipboardHistory.PasteType",
/*count=*/4);

// Wait for the clipboard buffer to be restored before performing another
// paste.
ClipboardDataWaiter().WaitFor(&clipboard_data);

// Open clipboard history and paste the last history item while holding down
// a non-shift key (arbitrarily, the ALT key). The item should not paste as
// plain text.
// Open clipboard history and paste the last history item via mouse click. The
// item should not paste as plain text.
ShowContextMenuViaAccelerator(/*wait_for_selection=*/true);
EXPECT_TRUE(GetClipboardHistoryController()->IsMenuShowing());
PressAndRelease(ui::KeyboardCode::VKEY_DOWN);
PressAndRelease(ui::KeyboardCode::VKEY_DOWN);
PressAndRelease(ui::KeyboardCode::VKEY_RETURN, ui::EF_ALT_DOWN);
const auto* menu_item_view =
GetContextMenu()->GetMenuItemViewAtForTest(/*index=*/2);
GetEventGenerator()->MoveMouseTo(
menu_item_view->GetBoundsInScreen().CenterPoint());
ASSERT_TRUE(menu_item_view->IsSelected());
GetEventGenerator()->ClickLeftButton();
EXPECT_FALSE(GetClipboardHistoryController()->IsMenuShowing());

// Wait for the paste event to propagate to the web contents.
// The web contents will notify us a paste occurred by updating page title.
std::ignore =
content::TitleWatcher(web_contents, u"Paste 4").WaitAndGetTitle();
content::TitleWatcher(web_contents, u"Paste 5").WaitAndGetTitle();

// Confirm the expected paste data.
last_paste = GetLastPaste();
ASSERT_EQ(last_paste.size(), 2u);
EXPECT_EQ(last_paste[0].GetString(), "text/plain: A");
EXPECT_EQ(last_paste[1].GetString(), "text/html: <span>A</span>");
histogram_tester.ExpectBucketCount("Ash.ClipboardHistory.PasteType",
ClipboardHistoryPasteType::kRichTextMouse,
/*expected_count=*/1);
histogram_tester.ExpectTotalCount("Ash.ClipboardHistory.PasteType",
/*count=*/5);

// Wait for the clipboard buffer to be restored before performing another
// paste.
ClipboardDataWaiter().WaitFor(&clipboard_data);

// Open clipboard history and paste the last history item via mouse click
// while holding down the shift key. The item should paste as plain text.
ShowContextMenuViaAccelerator(/*wait_for_selection=*/true);
EXPECT_TRUE(GetClipboardHistoryController()->IsMenuShowing());
menu_item_view = GetContextMenu()->GetMenuItemViewAtForTest(/*index=*/2);
GetEventGenerator()->MoveMouseTo(
menu_item_view->GetBoundsInScreen().CenterPoint());
ASSERT_TRUE(menu_item_view->IsSelected());
GetEventGenerator()->set_flags(ui::EF_SHIFT_DOWN);
GetEventGenerator()->ClickLeftButton();
GetEventGenerator()->set_flags(ui::EF_NONE);
EXPECT_FALSE(GetClipboardHistoryController()->IsMenuShowing());

// Wait for the paste event to propagate to the web contents.
// The web contents will notify us a paste occurred by updating page title.
std::ignore =
content::TitleWatcher(web_contents, u"Paste 6").WaitAndGetTitle();

// Confirm the expected paste data.
last_paste = GetLastPaste();
ASSERT_EQ(last_paste.size(), 1u);
EXPECT_EQ(last_paste[0].GetString(), "text/plain: A");
histogram_tester.ExpectBucketCount("Ash.ClipboardHistory.PasteType",
ClipboardHistoryPasteType::kPlainTextMouse,
/*expected_count=*/1);
histogram_tester.ExpectTotalCount("Ash.ClipboardHistory.PasteType",
/*count=*/6);

// Verify the clipboard buffer is restored to initial state.
ClipboardDataWaiter().WaitFor(&clipboard_data);
Expand Down Expand Up @@ -1082,19 +1176,28 @@ class ClipboardHistoryTextfieldBrowserTest
// correctly (https://crbug.com/1142088).
IN_PROC_BROWSER_TEST_F(ClipboardHistoryTextfieldBrowserTest,
VerifyResponseToGestures) {
base::HistogramTester histogram_tester;

SetClipboardText("A");
SetClipboardText("B");
ShowContextMenuViaAccelerator(/*wait_for_selection=*/true);
ASSERT_TRUE(GetClipboardHistoryController()->IsMenuShowing());

// Tap at the second menu item view. Verify that "A" is pasted.
histogram_tester.ExpectTotalCount("Ash.ClipboardHistory.PasteType",
/*count=*/0);
const views::MenuItemView* second_menu_item_view =
GetMenuItemViewForIndex(/*index=*/1);
GetEventGenerator()->GestureTapAt(
second_menu_item_view->GetBoundsInScreen().CenterPoint());
EXPECT_FALSE(GetClipboardHistoryController()->IsMenuShowing());
WaitForOperationConfirmed(/*success_expected=*/true);
EXPECT_EQ("A", base::UTF16ToUTF8(textfield_->GetText()));
histogram_tester.ExpectUniqueSample(
"Ash.ClipboardHistory.PasteType",
ash::ClipboardHistoryControllerImpl::ClipboardHistoryPasteType::
kRichTextTouch,
/*expected_bucket_count=*/1);
}

// Verifies that the metric to record the count of the consecutive pastes from
Expand Down

0 comments on commit e0c3f02

Please sign in to comment.