[OfflinePages] Classifies PRERENDERING_FAILED cases whether to TryNext

chrome/browser/android/offline_pages/prerendering_loader.cc

Index: chrome/browser/android/offline_pages/prerendering_loader.cc
diff --git a/chrome/browser/android/offline_pages/prerendering_loader.cc b/chrome/browser/android/offline_pages/prerendering_loader.cc
index c2b85f6e3060a79eb7d225ac13479a40e9ac1a92..fb1c4bc8c86abcbb0ca2615d0a2d0290ac0f56d1 100644
--- a/chrome/browser/android/offline_pages/prerendering_loader.cc
+++ b/chrome/browser/android/offline_pages/prerendering_loader.cc
@@ -34,22 +34,43 @@ namespace offline_pages {
 Offliner::RequestStatus ClassifyFinalStatus(
     prerender::FinalStatus final_status) {
   switch (final_status) {
-
-    // Identify aborted/canceled operations
+    // Identify aborted/canceled operations.
 
     case prerender::FINAL_STATUS_CANCELLED:
     // TODO(dougarnett): Reconsider if/when get better granularity (642768)
     case prerender::FINAL_STATUS_UNSUPPORTED_SCHEME:
       return Offliner::PRERENDERING_CANCELED;
 
-    // Identify non-retryable failues.
+    // Identify non-retryable failures. These are a hard type failures
+    // associated with the page and so are expected to occur again if retried.
 
     case prerender::FINAL_STATUS_SAFE_BROWSING:
     case prerender::FINAL_STATUS_CREATING_AUDIO_STREAM:
     case prerender::FINAL_STATUS_JAVASCRIPT_ALERT:
+    case prerender::FINAL_STATUS_CREATE_NEW_WINDOW:
+    case prerender::FINAL_STATUS_INVALID_HTTP_METHOD:
+    case prerender::FINAL_STATUS_OPEN_URL:
       return Offliner::RequestStatus::PRERENDERING_FAILED_NO_RETRY;
 
+    // Identify failures that indicate we should stop further processing
+    // for now. These may be current resource issues or app closing.
+
+    case prerender::FINAL_STATUS_MEMORY_LIMIT_EXCEEDED:
+    case prerender::FINAL_STATUS_RATE_LIMIT_EXCEEDED:
+    case prerender::FINAL_STATUS_RENDERER_CRASHED:
+    case prerender::FINAL_STATUS_TOO_MANY_PROCESSES:
+    case prerender::FINAL_STATUS_TIMED_OUT:
+    case prerender::FINAL_STATUS_APP_TERMINATING:
+    case prerender::FINAL_STATUS_PROFILE_DESTROYED:
+      return Offliner::RequestStatus::PRERENDERING_FAILED_NO_NEXT;

[Pete Williamson, 2016/12/02 23:06:09]

Should we add a test in prerendering loader unittest for this new return code?

[dougarnett, 2016/12/02 23:50:08]

Yes, thanks

+
     // Otherwise, assume retryable failure.
+
+    case prerender::FINAL_STATUS_NEW_NAVIGATION_ENTRY:
+    case prerender::FINAL_STATUS_CACHE_OR_HISTORY_CLEARED:
+    case prerender::FINAL_STATUS_SSL_ERROR:
+    case prerender::FINAL_STATUS_SSL_CLIENT_CERTIFICATE_REQUESTED:
+    case prerender::FINAL_STATUS_WINDOW_PRINT:
     default:
       return Offliner::RequestStatus::PRERENDERING_FAILED;
   }