Commit fc7c02ae authored by Michael van Ouwerkerk's avatar Michael van Ouwerkerk Committed by Commit Bot

Log Shared Clipboard send message retries with result.

Bug: 1042707
Change-Id: I6243c8dc6fd151738aac5c23e90968ab53f7e6cb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2002627Reviewed-by: default avatarRichard Knoll <knollr@chromium.org>
Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Michael van Ouwerkerk <mvanouwerkerk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732863}
parent 42eabe01
...@@ -59,11 +59,12 @@ public class SharingServiceProxy { ...@@ -59,11 +59,12 @@ public class SharingServiceProxy {
* @param lastUpdatedTimestampMillis The last updated timestamp in milliseconds of the receiver * @param lastUpdatedTimestampMillis The last updated timestamp in milliseconds of the receiver
* device. * device.
* @param text The text to send. * @param text The text to send.
* @param retries The number of retries so far.
* @param callback The result of the operation. Runs |callback| with a * @param callback The result of the operation. Runs |callback| with a
* org.chromium.chrome.browser.sharing.SharingSendMessageResult enum value. * org.chromium.chrome.browser.sharing.SharingSendMessageResult enum value.
*/ */
public void sendSharedClipboardMessage( public void sendSharedClipboardMessage(String guid, long lastUpdatedTimestampMillis,
String guid, long lastUpdatedTimestampMillis, String text, Callback<Integer> callback) { String text, int retries, Callback<Integer> callback) {
if (sNativeSharingServiceProxyAndroid == 0) { if (sNativeSharingServiceProxyAndroid == 0) {
callback.onResult(SharingSendMessageResult.INTERNAL_ERROR); callback.onResult(SharingSendMessageResult.INTERNAL_ERROR);
return; return;
...@@ -71,7 +72,7 @@ public class SharingServiceProxy { ...@@ -71,7 +72,7 @@ public class SharingServiceProxy {
Natives jni = SharingServiceProxyJni.get(); Natives jni = SharingServiceProxyJni.get();
jni.sendSharedClipboardMessage(sNativeSharingServiceProxyAndroid, guid, jni.sendSharedClipboardMessage(sNativeSharingServiceProxyAndroid, guid,
lastUpdatedTimestampMillis, text, callback); lastUpdatedTimestampMillis, text, retries, callback);
} }
/** /**
...@@ -132,7 +133,8 @@ public class SharingServiceProxy { ...@@ -132,7 +133,8 @@ public class SharingServiceProxy {
interface Natives { interface Natives {
void initSharingService(Profile profile); void initSharingService(Profile profile);
void sendSharedClipboardMessage(long nativeSharingServiceProxyAndroid, String guid, void sendSharedClipboardMessage(long nativeSharingServiceProxyAndroid, String guid,
long lastUpdatedTimestampMillis, String text, Callback<Integer> callback); long lastUpdatedTimestampMillis, String text, int retries,
Callback<Integer> callback);
void getDeviceCandidates(long nativeSharingServiceProxyAndroid, void getDeviceCandidates(long nativeSharingServiceProxyAndroid,
ArrayList<DeviceInfo> deviceInfo, int requiredFeature); ArrayList<DeviceInfo> deviceInfo, int requiredFeature);
void addDeviceCandidatesInitializedObserver( void addDeviceCandidatesInitializedObserver(
......
...@@ -32,6 +32,7 @@ public class SharedClipboardMessageHandler { ...@@ -32,6 +32,7 @@ public class SharedClipboardMessageHandler {
"SharedClipboard.EXTRA_DEVICE_CLIENT_NAME"; "SharedClipboard.EXTRA_DEVICE_CLIENT_NAME";
private static final String EXTRA_DEVICE_LAST_UPDATED_TIMESTAMP_MILLIS = private static final String EXTRA_DEVICE_LAST_UPDATED_TIMESTAMP_MILLIS =
"SharedClipboard.EXTRA_DEVICE_LAST_UPDATED_TIMESTAMP_MILLIS"; "SharedClipboard.EXTRA_DEVICE_LAST_UPDATED_TIMESTAMP_MILLIS";
private static final String EXTRA_RETRIES = "SharedClipboard.EXTRA_RETRIES";
/** /**
* Handles the tapping of an incoming notification when text is shared with current device. * Handles the tapping of an incoming notification when text is shared with current device.
...@@ -57,8 +58,9 @@ public class SharedClipboardMessageHandler { ...@@ -57,8 +58,9 @@ public class SharedClipboardMessageHandler {
long lastUpdatedTimestampMillis = IntentUtils.safeGetLongExtra( long lastUpdatedTimestampMillis = IntentUtils.safeGetLongExtra(
intent, EXTRA_DEVICE_LAST_UPDATED_TIMESTAMP_MILLIS, /*defaultValue=*/0); intent, EXTRA_DEVICE_LAST_UPDATED_TIMESTAMP_MILLIS, /*defaultValue=*/0);
String text = IntentUtils.safeGetStringExtra(intent, Intent.EXTRA_TEXT); String text = IntentUtils.safeGetStringExtra(intent, Intent.EXTRA_TEXT);
int retries = IntentUtils.safeGetIntExtra(intent, EXTRA_RETRIES, /*defaultValue=*/1);
showSendingNotification(guid, name, lastUpdatedTimestampMillis, text); showSendingNotification(guid, name, lastUpdatedTimestampMillis, text, retries);
} }
} }
...@@ -71,9 +73,10 @@ public class SharedClipboardMessageHandler { ...@@ -71,9 +73,10 @@ public class SharedClipboardMessageHandler {
* @param lastUpdatedTimestampMillis The last updated timestamp in milliseconds of the receiver * @param lastUpdatedTimestampMillis The last updated timestamp in milliseconds of the receiver
* device. * device.
* @param text The text shared from the sender device. * @param text The text shared from the sender device.
* @param retries The number of retries so far.
*/ */
public static void showSendingNotification( public static void showSendingNotification(
String guid, String name, long lastUpdatedTimestampMillis, String text) { String guid, String name, long lastUpdatedTimestampMillis, String text, int retries) {
if (TextUtils.isEmpty(guid) || TextUtils.isEmpty(name) || TextUtils.isEmpty(text)) { if (TextUtils.isEmpty(guid) || TextUtils.isEmpty(name) || TextUtils.isEmpty(text)) {
return; return;
} }
...@@ -92,7 +95,7 @@ public class SharedClipboardMessageHandler { ...@@ -92,7 +95,7 @@ public class SharedClipboardMessageHandler {
// TODO(crbug.com/1015411): Wait for device info in a more central place. // TODO(crbug.com/1015411): Wait for device info in a more central place.
SharingServiceProxy.getInstance().addDeviceCandidatesInitializedObserver(() -> { SharingServiceProxy.getInstance().addDeviceCandidatesInitializedObserver(() -> {
SharingServiceProxy.getInstance().sendSharedClipboardMessage( SharingServiceProxy.getInstance().sendSharedClipboardMessage(
guid, lastUpdatedTimestampMillis, text, result -> { guid, lastUpdatedTimestampMillis, text, retries, result -> {
if (result == SharingSendMessageResult.SUCCESSFUL) { if (result == SharingSendMessageResult.SUCCESSFUL) {
SharingNotificationUtil.dismissNotification( SharingNotificationUtil.dismissNotification(
NotificationConstants.GROUP_SHARED_CLIPBOARD, NotificationConstants.GROUP_SHARED_CLIPBOARD,
...@@ -112,7 +115,8 @@ public class SharedClipboardMessageHandler { ...@@ -112,7 +115,8 @@ public class SharedClipboardMessageHandler {
.putExtra(EXTRA_DEVICE_GUID, guid) .putExtra(EXTRA_DEVICE_GUID, guid)
.putExtra(EXTRA_DEVICE_CLIENT_NAME, name) .putExtra(EXTRA_DEVICE_CLIENT_NAME, name)
.putExtra(EXTRA_DEVICE_LAST_UPDATED_TIMESTAMP_MILLIS, .putExtra(EXTRA_DEVICE_LAST_UPDATED_TIMESTAMP_MILLIS,
lastUpdatedTimestampMillis), lastUpdatedTimestampMillis)
.putExtra(EXTRA_RETRIES, retries + 1),
PendingIntent.FLAG_UPDATE_CURRENT); PendingIntent.FLAG_UPDATE_CURRENT);
} }
......
...@@ -128,8 +128,8 @@ public class SharedClipboardShareActivity ...@@ -128,8 +128,8 @@ public class SharedClipboardShareActivity
SharedClipboardMetrics.recordDeviceClick(position); SharedClipboardMetrics.recordDeviceClick(position);
SharedClipboardMetrics.recordTextSize(text.length()); SharedClipboardMetrics.recordTextSize(text.length());
SharedClipboardMessageHandler.showSendingNotification( SharedClipboardMessageHandler.showSendingNotification(device.guid, device.clientName,
device.guid, device.clientName, device.lastUpdatedTimestampMillis, text); device.lastUpdatedTimestampMillis, text, /*retries=*/0);
finish(); finish();
} }
} }
...@@ -48,7 +48,7 @@ std::string DevicePlatformToString(SharingDevicePlatform device_platform) { ...@@ -48,7 +48,7 @@ std::string DevicePlatformToString(SharingDevicePlatform device_platform) {
// Maps SharingSendMessageResult enum values to strings used as histogram // Maps SharingSendMessageResult enum values to strings used as histogram
// suffixes. Keep in sync with "SharingSendMessageResult" in histograms.xml. // suffixes. Keep in sync with "SharingSendMessageResult" in histograms.xml.
std::string SharingSendMessageResultToSuffix(SharingSendMessageResult result) { std::string SendMessageResultToSuffix(SharingSendMessageResult result) {
switch (result) { switch (result) {
case SharingSendMessageResult::kSuccessful: case SharingSendMessageResult::kSuccessful:
return "Successful"; return "Successful";
...@@ -265,7 +265,7 @@ void LogSharingDeviceLastUpdatedAgeWithResult(SharingSendMessageResult result, ...@@ -265,7 +265,7 @@ void LogSharingDeviceLastUpdatedAgeWithResult(SharingSendMessageResult result,
base::TimeDelta age) { base::TimeDelta age) {
base::UmaHistogramCounts1000( base::UmaHistogramCounts1000(
base::StrCat({"Sharing.DeviceLastUpdatedAgeWithResult.", base::StrCat({"Sharing.DeviceLastUpdatedAgeWithResult.",
SharingSendMessageResultToSuffix(result)}), SendMessageResultToSuffix(result)}),
age.InHours()); age.InHours());
} }
...@@ -356,6 +356,14 @@ void LogSharedClipboardSelectedTextSize(size_t size) { ...@@ -356,6 +356,14 @@ void LogSharedClipboardSelectedTextSize(size_t size) {
size); size);
} }
void LogSharedClipboardRetries(int retries, SharingSendMessageResult result) {
constexpr char kBase[] = "Sharing.SharedClipboardRetries";
base::UmaHistogramExactLinear(kBase, retries, /*value_max=*/20);
base::UmaHistogramExactLinear(
base::StrCat({kBase, ".", SendMessageResultToSuffix(result)}), retries,
/*value_max=*/20);
}
void LogRemoteCopyHandleMessageResult(RemoteCopyHandleMessageResult result) { void LogRemoteCopyHandleMessageResult(RemoteCopyHandleMessageResult result) {
base::UmaHistogramEnumeration("Sharing.RemoteCopyHandleMessageResult", base::UmaHistogramEnumeration("Sharing.RemoteCopyHandleMessageResult",
result); result);
......
...@@ -148,6 +148,9 @@ void LogSendSharingAckMessageResult( ...@@ -148,6 +148,9 @@ void LogSendSharingAckMessageResult(
// Logs to UMA the size of the selected text for Shared Clipboard. // Logs to UMA the size of the selected text for Shared Clipboard.
void LogSharedClipboardSelectedTextSize(size_t text_size); void LogSharedClipboardSelectedTextSize(size_t text_size);
// Logs to UMA the number of retries for sending a Shared Clipboard message.
void LogSharedClipboardRetries(int retries, SharingSendMessageResult result);
// Logs to UMA the result of handling a Remote Copy message. // Logs to UMA the result of handling a Remote Copy message.
void LogRemoteCopyHandleMessageResult(RemoteCopyHandleMessageResult result); void LogRemoteCopyHandleMessageResult(RemoteCopyHandleMessageResult result);
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "chrome/browser/profiles/profile_android.h" #include "chrome/browser/profiles/profile_android.h"
#include "chrome/browser/sharing/features.h" #include "chrome/browser/sharing/features.h"
#include "chrome/browser/sharing/sharing_device_source.h" #include "chrome/browser/sharing/sharing_device_source.h"
#include "chrome/browser/sharing/sharing_metrics.h"
#include "chrome/browser/sharing/sharing_send_message_result.h" #include "chrome/browser/sharing/sharing_send_message_result.h"
#include "chrome/browser/sharing/sharing_service.h" #include "chrome/browser/sharing/sharing_service.h"
#include "chrome/browser/sharing/sharing_service_factory.h" #include "chrome/browser/sharing/sharing_service_factory.h"
...@@ -44,6 +45,7 @@ void SharingServiceProxyAndroid::SendSharedClipboardMessage( ...@@ -44,6 +45,7 @@ void SharingServiceProxyAndroid::SendSharedClipboardMessage(
const base::android::JavaParamRef<jstring>& j_guid, const base::android::JavaParamRef<jstring>& j_guid,
const jlong j_last_updated_timestamp_millis, const jlong j_last_updated_timestamp_millis,
const base::android::JavaParamRef<jstring>& j_text, const base::android::JavaParamRef<jstring>& j_text,
const jint j_retries,
const base::android::JavaParamRef<jobject>& j_runnable) { const base::android::JavaParamRef<jobject>& j_runnable) {
std::string guid = base::android::ConvertJavaStringToUTF8(env, j_guid); std::string guid = base::android::ConvertJavaStringToUTF8(env, j_guid);
DCHECK(!guid.empty()); DCHECK(!guid.empty());
...@@ -68,13 +70,14 @@ void SharingServiceProxyAndroid::SendSharedClipboardMessage( ...@@ -68,13 +70,14 @@ void SharingServiceProxyAndroid::SendSharedClipboardMessage(
device, base::TimeDelta::FromSeconds(kSharingMessageTTLSeconds.Get()), device, base::TimeDelta::FromSeconds(kSharingMessageTTLSeconds.Get()),
std::move(sharing_message), std::move(sharing_message),
base::BindOnce( base::BindOnce(
[](base::OnceCallback<void(int)> callback, [](int retries, base::OnceCallback<void(int)> callback,
SharingSendMessageResult result, SharingSendMessageResult result,
std::unique_ptr<chrome_browser_sharing::ResponseMessage> std::unique_ptr<chrome_browser_sharing::ResponseMessage>
response) { response) {
LogSharedClipboardRetries(retries, result);
std::move(callback).Run(static_cast<int>(result)); std::move(callback).Run(static_cast<int>(result));
}, },
std::move(callback))); j_retries, std::move(callback)));
} }
void SharingServiceProxyAndroid::GetDeviceCandidates( void SharingServiceProxyAndroid::GetDeviceCandidates(
......
...@@ -21,6 +21,7 @@ class SharingServiceProxyAndroid { ...@@ -21,6 +21,7 @@ class SharingServiceProxyAndroid {
const base::android::JavaParamRef<jstring>& j_guid, const base::android::JavaParamRef<jstring>& j_guid,
const jlong j_last_updated_timestamp_millis, const jlong j_last_updated_timestamp_millis,
const base::android::JavaParamRef<jstring>& j_text, const base::android::JavaParamRef<jstring>& j_text,
const jint j_retries,
const base::android::JavaParamRef<jobject>& j_runnable); const base::android::JavaParamRef<jobject>& j_runnable);
void GetDeviceCandidates( void GetDeviceCandidates(
......
...@@ -142658,6 +142658,19 @@ should be kept until we remove incident reporting. --> ...@@ -142658,6 +142658,19 @@ should be kept until we remove incident reporting. -->
</summary> </summary>
</histogram> </histogram>
<histogram name="Sharing.SharedClipboardRetries" units="retries"
expires_after="2020-06-15">
<!-- Name completed by histogram_suffixes name="SharingSendMessageResult" -->
<owner>mvanouwerkerk@chromium.org</owner>
<owner>knollr@chromium.org</owner>
<summary>
The retry count for sending a shared clipboard message. The zero value is
for the first attempt to send the message, which is not a retry. Logged when
the result of sending the message is known.
</summary>
</histogram>
<histogram name="Sharing.SharedClipboardSelectedDeviceIndex" units="index" <histogram name="Sharing.SharedClipboardSelectedDeviceIndex" units="index"
expires_after="2020-04-19"> expires_after="2020-04-19">
<owner>mvanouwerkerk@chromium.org</owner> <owner>mvanouwerkerk@chromium.org</owner>
...@@ -189264,6 +189277,7 @@ regressions. --> ...@@ -189264,6 +189277,7 @@ regressions. -->
<suffix name="PayloadTooLarge" label="Payload is too large"/> <suffix name="PayloadTooLarge" label="Payload is too large"/>
<suffix name="Successful" label="Successful"/> <suffix name="Successful" label="Successful"/>
<affected-histogram name="Sharing.DeviceLastUpdatedAgeWithResult"/> <affected-histogram name="Sharing.DeviceLastUpdatedAgeWithResult"/>
<affected-histogram name="Sharing.SharedClipboardRetries"/>
</histogram_suffixes> </histogram_suffixes>
<histogram_suffixes name="ShillCumulativeTimeOnline" separator="."> <histogram_suffixes name="ShillCumulativeTimeOnline" separator=".">
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