Skip to content

Commit

Permalink
Record time interval between clipboard data commit and data read.
Browse files Browse the repository at this point in the history
Will be used by PMs to analyze user behavior on CrOS in light of new
clipboard data requirements coming from Android.

Note that the histogram added in this CL is only implemented for CrOS'
`ClipboardNonBacked` clipboard implementation.

Bug: 1310400
Change-Id: I63fbb8fa70ddb36c580ed71abc97fb1ab42c7325
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3553622
Reviewed-by: Austin Sullivan <asully@chromium.org>
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Commit-Queue: David Black <dmblack@google.com>
Cr-Commit-Position: refs/heads/main@{#987040}
  • Loading branch information
David Black authored and Chromium LUCI CQ committed Mar 30, 2022
1 parent 671c336 commit c6da651
Show file tree
Hide file tree
Showing 9 changed files with 228 additions and 7 deletions.
1 change: 1 addition & 0 deletions ash/clipboard/clipboard_history_controller_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,7 @@ void ClipboardHistoryControllerImpl::PasteClipboardHistoryItem(
if (paste_plain_text) {
// When the shift key is pressed, we only paste plain text.
temp_data = std::make_unique<ui::ClipboardData>();
temp_data->set_commit_time(item.data().commit_time());
temp_data->set_text(item.data().text());
ui::DataTransferEndpoint* data_src = item.data().source();
if (data_src) {
Expand Down
13 changes: 13 additions & 0 deletions tools/metrics/histograms/metadata/storage/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,19 @@ chromium-metrics-reviews@google.com.
</summary>
</histogram>

<histogram name="Clipboard.TimeIntervalBetweenCommitAndRead" units="ms"
expires_after="2022-09-25">
<owner>dcheng@chromium.org</owner>
<owner>src/ui/base/clipboard/OWNERS</owner>
<summary>
Records the time interval between when clipboard data was committed to the
clipboard and the same clipboard data was read from the clipboard. The same
clipboard data may be read multiple times. Emitted at the moment the
clipboard data was read. Currently only implemented for Chrome OS' in-memory
clipboard (`ui::ClipboardNonBacked`).
</summary>
</histogram>

<histogram name="Clipboard.Write" enum="ClipboardFormatWrite"
expires_after="2022-08-07">
<owner>huangdarwin@chromium.org</owner>
Expand Down
3 changes: 3 additions & 0 deletions ui/base/clipboard/clipboard_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ ClipboardData::ClipboardData(const ClipboardData& other) {
filenames_ = other.filenames_;
src_ = other.src_ ? std::make_unique<DataTransferEndpoint>(*other.src_.get())
: nullptr;
#if BUILDFLAG(IS_CHROMEOS)
commit_time_ = other.commit_time_;
#endif // BUILDFLAG(IS_CHROMEOS)
}

ClipboardData::~ClipboardData() = default;
Expand Down
16 changes: 16 additions & 0 deletions ui/base/clipboard/clipboard_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
#include "ui/base/clipboard/file_info.h"
#include "ui/base/data_transfer_policy/data_transfer_endpoint.h"

#if BUILDFLAG(IS_CHROMEOS)
#include "base/time/time.h"
#endif // BUILDFLAG(IS_CHROMEOS)

class SkBitmap;

namespace ui {
Expand Down Expand Up @@ -151,6 +155,13 @@ class COMPONENT_EXPORT(UI_BASE_CLIPBOARD) ClipboardData {
src_ = std::move(src);
}

#if BUILDFLAG(IS_CHROMEOS)
absl::optional<base::Time> commit_time() const { return commit_time_; }
void set_commit_time(absl::optional<base::Time> commit_time) {
commit_time_ = commit_time;
}
#endif // BUILDFLAG(IS_CHROMEOS)

private:
// Plain text in UTF8 format.
std::string text_;
Expand Down Expand Up @@ -197,6 +208,11 @@ class COMPONENT_EXPORT(UI_BASE_CLIPBOARD) ClipboardData {

// The source of the data.
std::unique_ptr<DataTransferEndpoint> src_;

#if BUILDFLAG(IS_CHROMEOS)
// If present, the time at which this data was committed to the clipboard.
absl::optional<base::Time> commit_time_;
#endif // BUILDFLAG(IS_CHROMEOS)
};

} // namespace ui
Expand Down
20 changes: 20 additions & 0 deletions ui/base/clipboard/clipboard_metrics.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
#include "base/metrics/histogram_functions.h"
#include "base/metrics/histogram_macros.h"

#if BUILDFLAG(IS_CHROMEOS)
#include "ui/base/clipboard/clipboard_data.h"
#endif // BUILDFLAG(IS_CHROMEOS)

namespace ui {

void RecordRead(ClipboardFormatMetric metric) {
Expand All @@ -17,4 +21,20 @@ void RecordWrite(ClipboardFormatMetric metric) {
base::UmaHistogramEnumeration("Clipboard.Write", metric);
}

#if BUILDFLAG(IS_CHROMEOS)
void RecordTimeIntervalBetweenCommitAndRead(const ui::ClipboardData* data) {
if (!data)
return;

absl::optional<base::Time> commit_time = data->commit_time();
if (!commit_time.has_value())
return;

base::UmaHistogramCustomTimes("Clipboard.TimeIntervalBetweenCommitAndRead",
base::Time::Now() - commit_time.value(),
/*min=*/base::Milliseconds(1),
/*max=*/base::Hours(12), /*buckets=*/100);
}
#endif // BUILDFLAG(IS_CHROMEOS)

} // namespace ui
12 changes: 12 additions & 0 deletions ui/base/clipboard/clipboard_metrics.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,14 @@
#ifndef UI_BASE_CLIPBOARD_CLIPBOARD_METRICS_H_
#define UI_BASE_CLIPBOARD_CLIPBOARD_METRICS_H_

#include "build/build_config.h"

namespace ui {

#if BUILDFLAG(IS_CHROMEOS)
class ClipboardData;
#endif // BUILDFLAG(IS_CHROMEOS)

// Used to log formats read/written from/to the platform clipboard.
//
// This enum's values are persisted to logs. Do not reuse or renumber values.
Expand All @@ -28,6 +34,12 @@ enum class ClipboardFormatMetric {
void RecordRead(ClipboardFormatMetric metric);
void RecordWrite(ClipboardFormatMetric metric);

#if BUILDFLAG(IS_CHROMEOS)
// Records the time interval between when the specified |data| was committed to
// the clipboard and when it was read. Read time is assumed to be now.
void RecordTimeIntervalBetweenCommitAndRead(const ui::ClipboardData* data);
#endif // BUILDFLAG(IS_CHROMEOS)

} // namespace ui

#endif // UI_BASE_CLIPBOARD_CLIPBOARD_METRICS_H_
47 changes: 47 additions & 0 deletions ui/base/clipboard/clipboard_non_backed.cc
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,9 @@ class ClipboardDataBuilder {
ClipboardInternal* clipboard,
std::unique_ptr<DataTransferEndpoint> data_src) {
ClipboardData* data = GetCurrentData();
#if BUILDFLAG(IS_CHROMEOS)
data->set_commit_time(base::Time::Now());
#endif // BUILDFLAG(IS_CHROMEOS)
data->set_source(std::move(data_src));
clipboard->WriteData(TakeCurrentData());
}
Expand Down Expand Up @@ -587,6 +590,10 @@ void ClipboardNonBacked::ReadText(ClipboardBuffer buffer,
if (!clipboard_internal_->IsReadAllowed(data_dst))
return;

#if BUILDFLAG(IS_CHROMEOS)
RecordTimeIntervalBetweenCommitAndRead(clipboard_internal_->GetData());
#endif // BUILDFLAG(IS_CHROMEOS)

RecordRead(ClipboardFormatMetric::kText);
clipboard_internal_->ReadText(result);

Expand All @@ -603,6 +610,10 @@ void ClipboardNonBacked::ReadAsciiText(ClipboardBuffer buffer,
if (!clipboard_internal_->IsReadAllowed(data_dst))
return;

#if BUILDFLAG(IS_CHROMEOS)
RecordTimeIntervalBetweenCommitAndRead(clipboard_internal_->GetData());
#endif // BUILDFLAG(IS_CHROMEOS)

RecordRead(ClipboardFormatMetric::kText);
clipboard_internal_->ReadAsciiText(result);

Expand All @@ -622,6 +633,10 @@ void ClipboardNonBacked::ReadHTML(ClipboardBuffer buffer,
if (!clipboard_internal_->IsReadAllowed(data_dst))
return;

#if BUILDFLAG(IS_CHROMEOS)
RecordTimeIntervalBetweenCommitAndRead(clipboard_internal_->GetData());
#endif // BUILDFLAG(IS_CHROMEOS)

RecordRead(ClipboardFormatMetric::kHtml);
clipboard_internal_->ReadHTML(markup, src_url, fragment_start, fragment_end);

Expand All @@ -638,8 +653,16 @@ void ClipboardNonBacked::ReadSvg(ClipboardBuffer buffer,
if (!clipboard_internal_->IsReadAllowed(data_dst))
return;

#if BUILDFLAG(IS_CHROMEOS)
RecordTimeIntervalBetweenCommitAndRead(clipboard_internal_->GetData());
#endif // BUILDFLAG(IS_CHROMEOS)

RecordRead(ClipboardFormatMetric::kSvg);
clipboard_internal_->ReadSvg(result);

#if BUILDFLAG(IS_CHROMEOS_ASH)
ClipboardMonitor::GetInstance()->NotifyClipboardDataRead();
#endif
}

void ClipboardNonBacked::ReadRTF(ClipboardBuffer buffer,
Expand All @@ -650,6 +673,10 @@ void ClipboardNonBacked::ReadRTF(ClipboardBuffer buffer,
if (!clipboard_internal_->IsReadAllowed(data_dst))
return;

#if BUILDFLAG(IS_CHROMEOS)
RecordTimeIntervalBetweenCommitAndRead(clipboard_internal_->GetData());
#endif // BUILDFLAG(IS_CHROMEOS)

RecordRead(ClipboardFormatMetric::kRtf);
clipboard_internal_->ReadRTF(result);

Expand All @@ -668,6 +695,10 @@ void ClipboardNonBacked::ReadPng(ClipboardBuffer buffer,
return;
}

#if BUILDFLAG(IS_CHROMEOS)
RecordTimeIntervalBetweenCommitAndRead(clipboard_internal_->GetData());
#endif // BUILDFLAG(IS_CHROMEOS)

RecordRead(ClipboardFormatMetric::kPng);
clipboard_internal_->ReadPng(std::move(callback));

Expand All @@ -685,6 +716,10 @@ void ClipboardNonBacked::ReadCustomData(ClipboardBuffer buffer,
if (!clipboard_internal_->IsReadAllowed(data_dst))
return;

#if BUILDFLAG(IS_CHROMEOS)
RecordTimeIntervalBetweenCommitAndRead(clipboard_internal_->GetData());
#endif // BUILDFLAG(IS_CHROMEOS)

RecordRead(ClipboardFormatMetric::kCustomData);
clipboard_internal_->ReadCustomData(type, result);

Expand All @@ -702,6 +737,10 @@ void ClipboardNonBacked::ReadFilenames(
if (!clipboard_internal_->IsReadAllowed(data_dst))
return;

#if BUILDFLAG(IS_CHROMEOS)
RecordTimeIntervalBetweenCommitAndRead(clipboard_internal_->GetData());
#endif // BUILDFLAG(IS_CHROMEOS)

RecordRead(ClipboardFormatMetric::kFilenames);
*result = clipboard_internal_->ReadFilenames();

Expand All @@ -718,6 +757,10 @@ void ClipboardNonBacked::ReadBookmark(const DataTransferEndpoint* data_dst,
if (!clipboard_internal_->IsReadAllowed(data_dst))
return;

#if BUILDFLAG(IS_CHROMEOS)
RecordTimeIntervalBetweenCommitAndRead(clipboard_internal_->GetData());
#endif // BUILDFLAG(IS_CHROMEOS)

RecordRead(ClipboardFormatMetric::kBookmark);
clipboard_internal_->ReadBookmark(title, url);

Expand All @@ -734,6 +777,10 @@ void ClipboardNonBacked::ReadData(const ClipboardFormatType& format,
if (!clipboard_internal_->IsReadAllowed(data_dst))
return;

#if BUILDFLAG(IS_CHROMEOS)
RecordTimeIntervalBetweenCommitAndRead(clipboard_internal_->GetData());
#endif // BUILDFLAG(IS_CHROMEOS)

RecordRead(ClipboardFormatMetric::kData);
clipboard_internal_->ReadData(format.GetName(), result);

Expand Down
4 changes: 3 additions & 1 deletion ui/base/clipboard/clipboard_non_backed.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,13 @@ class COMPONENT_EXPORT(UI_BASE_CLIPBOARD) ClipboardNonBacked

private:
friend class Clipboard;
friend class ClipboardNonBackedTest;
friend class ClipboardNonBackedTestBase;
FRIEND_TEST_ALL_PREFIXES(ClipboardNonBackedTest, TextURIList);
FRIEND_TEST_ALL_PREFIXES(ClipboardNonBackedTest, ImageEncoding);
FRIEND_TEST_ALL_PREFIXES(ClipboardNonBackedTest, EncodeImageOnce);
FRIEND_TEST_ALL_PREFIXES(ClipboardNonBackedTest, EncodeMultipleImages);
FRIEND_TEST_ALL_PREFIXES(ClipboardNonBackedMockTimeTest,
RecordsTimeIntervalBetweenCommitAndRead);
ClipboardNonBacked();
~ClipboardNonBacked() override;

Expand Down

0 comments on commit c6da651

Please sign in to comment.