[AndroidSms] Add metrics for FcmConnectionEstablisher

This CL adds metrics to FcmConnectionEstablisher class to track retries
and failures. See go/awm-cros-fcm for details.

Bug: 926314
Change-Id: I6089fbb5e44a8b2c69fd20b787214378014ed216
Reviewed-on: https://chromium-review.googlesource.com/c/1443896
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Jeremy Klein <jlklein@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Azeem Arshad <azeemarshad@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#628049}(cherry picked from commit 3edbb0ea725d7b4cfc3871dc3a5fd00bcfd62a9b)
Reviewed-on: https://chromium-review.googlesource.com/c/1450742
Cr-Commit-Position: refs/branch-heads/3683@{#140}
Cr-Branched-From: e51029943e0a38dd794b73caaf6373d5496ae783-refs/heads/master@{#625896}
diff --git a/chrome/browser/chromeos/android_sms/fcm_connection_establisher.cc b/chrome/browser/chromeos/android_sms/fcm_connection_establisher.cc
index ba560be..5d68e47 100644
--- a/chrome/browser/chromeos/android_sms/fcm_connection_establisher.cc
+++ b/chrome/browser/chromeos/android_sms/fcm_connection_establisher.cc
@@ -6,6 +6,7 @@
 
 #include <utility>
 
+#include "base/metrics/histogram_macros.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/post_task.h"
 #include "chromeos/components/multidevice/logging/logging.h"
@@ -41,10 +42,10 @@
 FcmConnectionEstablisher::PendingServiceWorkerMessage::
     PendingServiceWorkerMessage(
         GURL service_worker_scope,
-        std::string message_content,
+        MessageType message_type,
         content::ServiceWorkerContext* service_worker_context)
     : service_worker_scope(service_worker_scope),
-      message_content(message_content),
+      message_type(message_type),
       service_worker_context(service_worker_context) {}
 
 FcmConnectionEstablisher::InFlightMessage::InFlightMessage(
@@ -65,7 +66,7 @@
       base::BindOnce(
           &FcmConnectionEstablisher::SendMessageToServiceWorkerWithRetries,
           weak_ptr_factory_.GetWeakPtr(), url,
-          GetMessageForConnectionMode(connection_mode),
+          GetMessageTypeForConnectionMode(connection_mode),
           service_worker_context));
 }
 
@@ -76,18 +77,33 @@
       FROM_HERE, {content::BrowserThread::IO},
       base::BindOnce(
           &FcmConnectionEstablisher::SendMessageToServiceWorkerWithRetries,
-          weak_ptr_factory_.GetWeakPtr(), url, kStopFcmMessage,
+          weak_ptr_factory_.GetWeakPtr(), url, MessageType::kStop,
           service_worker_context));
 }
 
 // static
-std::string FcmConnectionEstablisher::GetMessageForConnectionMode(
+FcmConnectionEstablisher::MessageType
+FcmConnectionEstablisher::GetMessageTypeForConnectionMode(
     ConnectionMode connection_mode) {
   switch (connection_mode) {
     case ConnectionMode::kStartConnection:
-      return kStartFcmMessage;
+      return MessageType::kStart;
     case ConnectionMode::kResumeExistingConnection:
+      return MessageType::kResume;
+  }
+  NOTREACHED();
+}
+
+// static
+std::string FcmConnectionEstablisher::GetMessageStringForType(
+    MessageType message_type) {
+  switch (message_type) {
+    case MessageType::kStart:
+      return kStartFcmMessage;
+    case MessageType::kResume:
       return kResumeFcmMessage;
+    case MessageType::kStop:
+      return kStopFcmMessage;
   }
   NOTREACHED();
   return "";
@@ -95,11 +111,11 @@
 
 void FcmConnectionEstablisher::SendMessageToServiceWorkerWithRetries(
     const GURL& url,
-    std::string message_string,
+    MessageType message_type,
     content::ServiceWorkerContext* service_worker_context) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
-  message_queue_.emplace(url, message_string, service_worker_context);
+  message_queue_.emplace(url, message_type, service_worker_context);
   ProcessMessageQueue();
 }
 
@@ -119,11 +135,11 @@
 void FcmConnectionEstablisher::SendInFlightMessage() {
   const PendingServiceWorkerMessage& message = in_flight_message_->message;
   blink::TransferableMessage msg;
-  msg.owned_encoded_message =
-      blink::EncodeStringMessage(base::UTF8ToUTF16(message.message_content));
+  msg.owned_encoded_message = blink::EncodeStringMessage(
+      base::UTF8ToUTF16(GetMessageStringForType(message.message_type)));
   msg.encoded_message = msg.owned_encoded_message;
 
-  PA_LOG(VERBOSE) << "Dispatching message " << message.message_content;
+  PA_LOG(VERBOSE) << "Dispatching message " << message.message_type;
   message.service_worker_context->StartServiceWorkerAndDispatchMessage(
       message.service_worker_scope, std::move(msg),
       base::BindOnce(&FcmConnectionEstablisher::OnMessageDispatchResult,
@@ -133,12 +149,16 @@
 void FcmConnectionEstablisher::OnMessageDispatchResult(bool status) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   DCHECK(in_flight_message_);
-  PA_LOG(VERBOSE) << "Service worker message returned status: " << status;
+  PA_LOG(VERBOSE) << "Service worker message returned status " << status
+                  << " for message "
+                  << in_flight_message_->message.message_type;
 
   if (!status && in_flight_message_->retry_count < kMaxRetryCount) {
     base::TimeDelta retry_delay =
         kRetryDelay * (1 << in_flight_message_->retry_count);
     in_flight_message_->retry_count++;
+    UMA_HISTOGRAM_ENUMERATION("AndroidSms.FcmMessageDispatchRetry",
+                              in_flight_message_->message.message_type);
     PA_LOG(VERBOSE) << "Scheduling retry with delay " << retry_delay;
     retry_timer_->Start(
         FROM_HERE, retry_delay,
@@ -147,15 +167,37 @@
     return;
   }
 
-  if (in_flight_message_->retry_count >= kMaxRetryCount) {
+  if (status) {
+    UMA_HISTOGRAM_ENUMERATION("AndroidSms.FcmMessageDispatchSuccess",
+                              in_flight_message_->message.message_type);
+  } else {
+    UMA_HISTOGRAM_ENUMERATION("AndroidSms.FcmMessageDispatchFailure",
+                              in_flight_message_->message.message_type);
     PA_LOG(WARNING) << "Max retries attempted when dispatching message "
-                    << in_flight_message_->message.message_content;
+                    << in_flight_message_->message.message_type;
   }
 
   in_flight_message_.reset();
   ProcessMessageQueue();
 }
 
+std::ostream& operator<<(
+    std::ostream& stream,
+    const FcmConnectionEstablisher::MessageType& message_type) {
+  switch (message_type) {
+    case FcmConnectionEstablisher::MessageType::kStart:
+      stream << "MessageType::kStart";
+      break;
+    case FcmConnectionEstablisher::MessageType::kResume:
+      stream << "MessageType::kResume";
+      break;
+    case FcmConnectionEstablisher::MessageType::kStop:
+      stream << "MessageType::kStop";
+      break;
+  }
+  return stream;
+}
+
 }  // namespace android_sms
 
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/android_sms/fcm_connection_establisher.h b/chrome/browser/chromeos/android_sms/fcm_connection_establisher.h
index 70905ee..926623a 100644
--- a/chrome/browser/chromeos/android_sms/fcm_connection_establisher.h
+++ b/chrome/browser/chromeos/android_sms/fcm_connection_establisher.h
@@ -36,13 +36,24 @@
       content::ServiceWorkerContext* service_worker_context) override;
 
  private:
+  // This enum is used for logging metrics. It should be kept in sync with
+  // AndroidSmsFcmMessageType in enums.xml
+  enum class MessageType {
+    kStart = 0,
+    kResume = 1,
+    kStop = 2,
+    kMaxValue = kStop,
+  };
+  friend std::ostream& operator<<(std::ostream& stream,
+                                  const MessageType& message_type);
+
   struct PendingServiceWorkerMessage {
     PendingServiceWorkerMessage(
         GURL service_worker_scope,
-        std::string message_content,
+        MessageType message_type,
         content::ServiceWorkerContext* service_worker_context);
     GURL service_worker_scope;
-    std::string message_content;
+    MessageType message_type;
     content::ServiceWorkerContext* service_worker_context;
   };
 
@@ -57,12 +68,14 @@
   FRIEND_TEST_ALL_PREFIXES(FcmConnectionEstablisherTest,
                            TestTearDownConnection);
 
-  static std::string GetMessageForConnectionMode(
+  static MessageType GetMessageTypeForConnectionMode(
       ConnectionMode connection_mode);
 
+  static std::string GetMessageStringForType(MessageType message_type);
+
   void SendMessageToServiceWorkerWithRetries(
       const GURL& url,
-      std::string message_string,
+      MessageType message_type,
       content::ServiceWorkerContext* service_worker_context);
 
   void ProcessMessageQueue();
diff --git a/chrome/browser/chromeos/android_sms/fcm_connection_establisher_unittest.cc b/chrome/browser/chromeos/android_sms/fcm_connection_establisher_unittest.cc
index bb382db..12c95b7 100644
--- a/chrome/browser/chromeos/android_sms/fcm_connection_establisher_unittest.cc
+++ b/chrome/browser/chromeos/android_sms/fcm_connection_establisher_unittest.cc
@@ -8,6 +8,7 @@
 
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/metrics/histogram_tester.h"
 #include "base/timer/mock_timer.h"
 #include "chrome/browser/chromeos/android_sms/android_sms_urls.h"
 #include "content/public/test/fake_service_worker_context.h"
@@ -44,6 +45,7 @@
 TEST_F(FcmConnectionEstablisherTest, TestEstablishConnection) {
   auto mock_retry_timer = std::make_unique<base::MockOneShotTimer>();
   base::MockOneShotTimer* mock_retry_timer_ptr = mock_retry_timer.get();
+  base::HistogramTester histogram_tester;
 
   content::FakeServiceWorkerContext fake_service_worker_context;
   FcmConnectionEstablisher fcm_connection_establisher(
@@ -64,11 +66,15 @@
                              message_dispatch_calls[0]);
 
   // Return success to result callback and verify that no retries are attempted
+  // and success histogram is recorded.
   std::move(std::get<content::ServiceWorkerContext::ResultCallback>(
                 message_dispatch_calls[0]))
       .Run(true /* status */);
   ASSERT_EQ(1u, message_dispatch_calls.size());
   EXPECT_FALSE(mock_retry_timer_ptr->IsRunning());
+  histogram_tester.ExpectBucketCount(
+      "AndroidSms.FcmMessageDispatchSuccess",
+      FcmConnectionEstablisher::MessageType::kStart, 1);
 
   // Verify that when multiple requests are sent only the first one is
   // dispatched while the others are queued.
@@ -91,6 +97,10 @@
       .Run(false /* status */);
   ASSERT_EQ(2u, message_dispatch_calls.size());
   EXPECT_TRUE(mock_retry_timer_ptr->IsRunning());
+  // Retry shouldn't count success.
+  histogram_tester.ExpectBucketCount(
+      "AndroidSms.FcmMessageDispatchSuccess",
+      FcmConnectionEstablisher::MessageType::kStart, 1);
   mock_retry_timer_ptr->Fire();
   ASSERT_EQ(3u, message_dispatch_calls.size());
   VerifyTransferrableMessage(FcmConnectionEstablisher::kStartFcmMessage,
@@ -119,6 +129,11 @@
       &fake_service_worker_context);
   base::RunLoop().RunUntilIdle();
 
+  int last_retry_bucket_count = histogram_tester.GetBucketCount(
+      "AndroidSms.FcmMessageDispatchRetry",
+      static_cast<base::HistogramBase::Sample>(
+          FcmConnectionEstablisher::MessageType::kStart));
+
   int retry_count = 0;
   while (true) {
     ASSERT_EQ(5u + retry_count, message_dispatch_calls.size());
@@ -134,6 +149,13 @@
   }
 
   EXPECT_EQ(FcmConnectionEstablisher::kMaxRetryCount, retry_count);
+  histogram_tester.ExpectBucketCount(
+      "AndroidSms.FcmMessageDispatchRetry",
+      FcmConnectionEstablisher::MessageType::kStart,
+      retry_count + last_retry_bucket_count);
+  histogram_tester.ExpectBucketCount(
+      "AndroidSms.FcmMessageDispatchFailure",
+      FcmConnectionEstablisher::MessageType::kStart, 1);
 }
 
 TEST_F(FcmConnectionEstablisherTest, TestTearDownConnection) {
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
index 03c7022..29c305e 100644
--- a/tools/metrics/histograms/enums.xml
+++ b/tools/metrics/histograms/enums.xml
@@ -1205,6 +1205,12 @@
   </int>
 </enum>
 
+<enum name="AndroidSmsFcmMessageType">
+  <int value="0" label="Start"/>
+  <int value="1" label="Resume"/>
+  <int value="2" label="Stop"/>
+</enum>
+
 <enum name="AndroidTabCloseUndoToastEvent">
   <int value="0" label="Undo Shown (Cold)"/>
   <int value="1" label="Undo Shown (Warm)"/>
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index c8270e6..3a3d352 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -2979,6 +2979,36 @@
   </summary>
 </histogram>
 
+<histogram name="AndroidSms.FcmMessageDispatchFailure"
+    enum="AndroidSmsFcmMessageType">
+  <owner>azeemarshad@chromium.org</owner>
+  <summary>
+    Records message types for which all retry attempts failed when dispatching
+    to Android Messages for Web Service-Worker. This is recorded when using FCM
+    web push for background notificaitons.
+  </summary>
+</histogram>
+
+<histogram name="AndroidSms.FcmMessageDispatchRetry"
+    enum="AndroidSmsFcmMessageType">
+  <owner>azeemarshad@chromium.org</owner>
+  <summary>
+    Records message types for which a retry was attempted when dispatching to
+    Android Messages for Web Service-Worker. This is recorded when using FCM web
+    push for background notificaitons.
+  </summary>
+</histogram>
+
+<histogram name="AndroidSms.FcmMessageDispatchSuccess"
+    enum="AndroidSmsFcmMessageType">
+  <owner>azeemarshad@chromium.org</owner>
+  <summary>
+    Records message types for which dispatching to Android Messages for Web
+    Service-Worker succeeded. This is recorded when using FCM web push for
+    background notificaitons.
+  </summary>
+</histogram>
+
 <histogram name="AndroidSms.MultiDeviceFeatureState"
     enum="MultiDevice_FeatureState">
   <owner>jlklein@chromium.org</owner>