[Offline Pages] Adds event logs for requests dropped due to number of start or complete attempts.

components/offline_pages/background/request_coordinator_event_logger.cc

Index: components/offline_pages/background/request_coordinator_event_logger.cc
diff --git a/components/offline_pages/background/request_coordinator_event_logger.cc b/components/offline_pages/background/request_coordinator_event_logger.cc
index 08702920a734dcb463a7c53df1d9a3cf38145bbb..1376b72d2d12cbaf5eb213a9df1ad32a9497a660 100644
--- a/components/offline_pages/background/request_coordinator_event_logger.cc
+++ b/components/offline_pages/background/request_coordinator_event_logger.cc
@@ -33,6 +33,33 @@ static std::string OfflinerRequestStatusToString(
   }
 }
 
+static std::string BackgroundSavePageResultToString(

[Pete Williamson, 2016/09/09 17:16:19]

This is a bit fragile - what happens if we add a new enum value?  At the least,
we should have a comment where BackgroundSavePageResult is defined that this
needs to be updated if we add a new value.  At best, there may be another way to
solve this that can't get out of sync.

Whatever solution we apply should also be applied to
OfflinerRequestStatusToString while we are at it.

[dougarnett, 2016/09/09 23:58:21]

How about just returning the int value of enum with no specific case?
Drop the DCHECKs?

[Pete Williamson, 2016/09/10 00:14:13]

that's OK, but it won't make the logging very legible.  I think I'd prefer
adding a comment in RequestNotifier::BackgroundSavePageResult.

[dougarnett, 2016/09/12 18:31:38]

Done

+    RequestNotifier::BackgroundSavePageResult result) {
+  switch (result) {
+    case RequestNotifier::BackgroundSavePageResult::SUCCESS:
+      return "SUCCESS";
+    case RequestNotifier::BackgroundSavePageResult::PRERENDER_FAILURE:
+      return "PRERENDER_FAILURE";
+    case RequestNotifier::BackgroundSavePageResult::PRERENDER_CANCELED:
+      return "PRERENDER_CANCELED";
+    case RequestNotifier::BackgroundSavePageResult::FOREGROUND_CANCELED:
+      return "FOREGROUND_CANCELED";
+    case RequestNotifier::BackgroundSavePageResult::SAVE_FAILED:
+      return "SAVE_FAILED";
+    case RequestNotifier::BackgroundSavePageResult::EXPIRED:
+      return "EXPIRED";
+    case RequestNotifier::BackgroundSavePageResult::RETRY_COUNT_EXCEEDED:
+      return "RETRY_COUNT_EXCEEDED";
+    case RequestNotifier::BackgroundSavePageResult::START_COUNT_EXCEEDED:
+      return "START_COUNT_EXCEEDED";
+    case RequestNotifier::BackgroundSavePageResult::REMOVED:
+      return "REMOVED";
+    default:
+      DCHECK(false);
+      return "";
+  }
+}
+
 static std::string UpdateRequestResultToString(
     RequestQueue::UpdateRequestResult result) {
   switch (result) {
@@ -50,21 +77,29 @@ static std::string UpdateRequestResultToString(
 
 }  // namespace
 
-void RequestCoordinatorEventLogger::RecordSavePageRequestUpdated(
+void RequestCoordinatorEventLogger::RecordOfflinerResult(
     const std::string& name_space,
     Offliner::RequestStatus new_status,
-    int64_t id) {
-  RecordActivity("Save page request for ID: " + std::to_string(id) +
-                 " and namespace: " + name_space +
-                 " has been updated with status " +
+    int64_t request_id) {
+  RecordActivity("Background save attempt for " + name_space + ":" +
+                 std::to_string(request_id) + " - " +
                  OfflinerRequestStatusToString(new_status));
 }
 
+void RequestCoordinatorEventLogger::RecordDroppedSavePageRequest(
+    const std::string& name_space,
+    RequestNotifier::BackgroundSavePageResult result,
+    int64_t request_id) {
+  RecordActivity("Background save request removed " + name_space + ":" +
+                 std::to_string(request_id) + " - " +
+                 BackgroundSavePageResultToString(result));
+}
+
 void RequestCoordinatorEventLogger::RecordUpdateRequestFailed(
     const std::string& name_space,
     RequestQueue::UpdateRequestResult result) {
-  RecordActivity("Updating queued request for namespace: " + name_space +
-                 " failed with result: " + UpdateRequestResultToString(result));
+  RecordActivity("Updating queued request for " + name_space + " failed - " +
+                 UpdateRequestResultToString(result));
 }
 
 }  // namespace offline_pages