[OfflinePages] Classifies PRERENDERING_FAILED cases whether to TryNext

chrome/browser/android/offline_pages/prerendering_loader.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/android/offline_pages/prerendering_loader.h"
 
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ skipping 16 matching lines @@
 
 
 namespace offline_pages {
 
 
 // Classifies the appropriate RequestStatus for for the given prerender
 // FinalStatus.
 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;
   }
 }
 
 PrerenderingLoader::PrerenderingLoader(content::BrowserContext* browser_context)
     : state_(State::IDLE),
       snapshot_controller_(nullptr),
       browser_context_(browser_context) {
   adapter_.reset(new PrerenderAdapter(this));
@@ skipping 174 matching lines @@
 void PrerenderingLoader::CancelPrerender() {
   if (adapter_->IsActive()) {
     adapter_->DestroyActive();
   }
   snapshot_controller_.reset(nullptr);
   session_contents_.reset(nullptr);
   state_ = State::IDLE;
 }
 
 }  // namespace offline_pages