[IOS Reading List] Change handling of distillation error.

ios/chrome/browser/reading_list/reading_list_download_service.cc

Index: ios/chrome/browser/reading_list/reading_list_download_service.cc
diff --git a/ios/chrome/browser/reading_list/reading_list_download_service.cc b/ios/chrome/browser/reading_list/reading_list_download_service.cc
index 7b9c4f5901d12c4124b2354d5eb0ca4d56b397cc..35a68eddf9684a568f16da87de2c9b2b78ebc10e 100644
--- a/ios/chrome/browser/reading_list/reading_list_download_service.cc
+++ b/ios/chrome/browser/reading_list/reading_list_download_service.cc
@@ -252,26 +252,31 @@ void ReadingListDownloadService::OnDownloadEnd(
     UMA_HISTOGRAM_ENUMERATION("ReadingList.Download.Status", SUCCESS,
                               STATUS_MAX);
 
-  } else if (success == URLDownloader::ERROR_RETRY) {
-    reading_list_model_->SetEntryDistilledState(url,
-                                                ReadingListEntry::WILL_RETRY);
-    ScheduleDownloadEntry(url);
-
+  } else if (success == URLDownloader::ERROR) {
     const ReadingListEntry* entry = reading_list_model_->GetEntryByURL(url);
+    bool will_retry = false;
     if (entry) {
-      if (entry->FailedDownloadCounter() < kNumberOfFailsBeforeStop) {
+      // Add this failure to the total failure count.
+      if (entry->FailedDownloadCounter() + 1 < kNumberOfFailsBeforeStop) {
         UMA_HISTOGRAM_ENUMERATION("ReadingList.Download.Status", RETRY,
                                   STATUS_MAX);
+        will_retry = true;
       } else {
         UMA_HISTOGRAM_ENUMERATION("ReadingList.Download.Status", FAILURE,
                                   STATUS_MAX);
       }
     }
+    if (will_retry) {

[gambard, 2017/02/17 14:07:47]

Why this is not merged with the metrics?
Is there a reason to schedule the download/set the state of a non-existing
entry?

[Olivier, 2017/02/17 14:27:15]

It is not merged precisely to not retry if entry does not exist :)
Merged using && precedence

+      reading_list_model_->SetEntryDistilledState(url,
+                                                  ReadingListEntry::WILL_RETRY);
+      ScheduleDownloadEntry(url);
+    } else {
+      reading_list_model_->SetEntryDistilledState(url, ReadingListEntry::ERROR);
+    }
 
-  } else if (success == URLDownloader::ERROR_PERMANENT) {
-    reading_list_model_->SetEntryDistilledState(url, ReadingListEntry::ERROR);
-    UMA_HISTOGRAM_ENUMERATION("ReadingList.Download.Status", FAILURE,
-                              STATUS_MAX);
+  } else {
+    // Catch if another status is added.

[gambard, 2017/02/17 14:07:47]

Use a switch?

[Olivier, 2017/02/17 14:27:15]

Done.

+    NOTREACHED();
   }
 }