[Offline Pages] Add a timer to BackgroundLoaderOffliner to delay SavePage by 2 seconds.

chrome/browser/android/offline_pages/background_loader_offliner.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/background_loader_offliner.h"
 
 #include "base/metrics/histogram_macros.h"
 #include "base/sys_info.h"
 #include "chrome/browser/android/offline_pages/offline_page_mhtml_archiver.h"
 #include "chrome/browser/android/offline_pages/offliner_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/offline_pages/core/background/save_page_request.h"
 #include "components/offline_pages/core/client_namespace_constants.h"
 #include "components/offline_pages/core/offline_page_model.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents.h"
 
 namespace offline_pages {
 
+namespace {
+long kOfflinePageDelayMs = 2000;

[Pete Williamson, 2017/02/24 17:31:45]

As a follow up change, let's put this into OfflinerPolicy, and use the constant
in both offliners.

[chili, 2017/02/24 21:43:47]

Acknowledged.

+}  // namespace
+
 BackgroundLoaderOffliner::BackgroundLoaderOffliner(
     content::BrowserContext* browser_context,
     const OfflinerPolicy* policy,
     OfflinePageModel* offline_page_model)
     : browser_context_(browser_context),
       offline_page_model_(offline_page_model),
       is_low_end_device_(base::SysInfo::IsLowEndDevice()),
       save_state_(NONE),
       page_load_state_(SUCCESS),
       weak_ptr_factory_(this) {
@@ skipping 56 matching lines @@
   }
 
   if (!OfflinePageModel::CanSaveURL(request.url())) {
     DVLOG(1) << "Not able to save page for requested url: " << request.url();
     return false;
   }
 
   if (!loader_)
     ResetState();
 
+  // Invalidate ptrs for all delayed/saving tasks.
+  weak_ptr_factory_.InvalidateWeakPtrs();
+
   // Track copy of pending request.
   pending_request_.reset(new SavePageRequest(request));
   completion_callback_ = callback;
 
   // Listen for app foreground/background change.
   app_listener_.reset(new base::android::ApplicationStatusListener(
       base::Bind(&BackgroundLoaderOffliner::OnApplicationStateChange,
                  weak_ptr_factory_.GetWeakPtr())));
 
   // Load page attempt.
@@ skipping 15 matching lines @@
 
   ResetState();
 }
 
 void BackgroundLoaderOffliner::DidStopLoading() {
   if (!pending_request_.get()) {
     DVLOG(1) << "DidStopLoading called even though no pending request.";
     return;
   }
 
-  SavePageRequest request(*pending_request_.get());

[Pete Williamson, 2017/02/24 17:31:45]

How about leaving this code where it is instead of moving into the delayed save
page?  If we had a navigation error, might as well take care of it now instead
of two seconds later.

[chili, 2017/02/24 21:43:47]

I think we still need this check in SavePage because a redirect can also result
in an error page.  While we do our best to ensure that pointers are invalidated,
there may be some race we're not aware of that results in an error page being
loaded.

-  // If there was an error navigating to page, return loading failed.
-  if (page_load_state_ != SUCCESS) {
-    Offliner::RequestStatus status =
-        (page_load_state_ == RETRIABLE)
-            ? Offliner::RequestStatus::LOADING_FAILED
-            : Offliner::RequestStatus::LOADING_FAILED_NO_RETRY;
-    completion_callback_.Run(request, status);
-    ResetState();
-    return;
-  }
+  // Invalidate ptrs for any ongoing save operation.
+  weak_ptr_factory_.InvalidateWeakPtrs();
 
-  save_state_ = SAVING;
-  content::WebContents* web_contents(
-      content::WebContentsObserver::web_contents());
-
-  std::unique_ptr<OfflinePageArchiver> archiver(
-      new OfflinePageMHTMLArchiver(web_contents));
-
-  OfflinePageModel::SavePageParams params;
-  params.url = web_contents->GetLastCommittedURL();
-  params.client_id = request.client_id();
-  params.proposed_offline_id = request.request_id();
-  params.is_background = true;
-
-  // Pass in the original URL if it's different from last committed
-  // when redirects occur.
-  if (params.url != request.url())
-    params.original_url = request.url();
-
-  offline_page_model_->SavePage(
-      params, std::move(archiver),
-      base::Bind(&BackgroundLoaderOffliner::OnPageSaved,
-                 weak_ptr_factory_.GetWeakPtr()));
+  // Post SavePage task with 2 second delay.
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&BackgroundLoaderOffliner::SavePage,
+                 weak_ptr_factory_.GetWeakPtr()),
+      base::TimeDelta::FromMilliseconds(kOfflinePageDelayMs));
 }
 
 void BackgroundLoaderOffliner::RenderProcessGone(
     base::TerminationStatus status) {
   if (pending_request_) {
     SavePageRequest request(*pending_request_.get());
     switch (status) {
       case base::TERMINATION_STATUS_OOM:
       case base::TERMINATION_STATUS_PROCESS_CRASHED:
       case base::TERMINATION_STATUS_STILL_RUNNING:
@@ skipping 49 matching lines @@
       case net::ERR_SSL_SERVER_CERT_BAD_FORMAT:
       case net::ERR_UNKNOWN_URL_SCHEME:
         page_load_state_ = NONRETRIABLE;
         break;
       default:
         page_load_state_ = RETRIABLE;
     }
   }
 }
 
+void BackgroundLoaderOffliner::SavePage() {
+  if (!pending_request_.get()) {
+    DVLOG(1) << "Pending request was cleared during delay.";
+    return;
+  }
+
+  SavePageRequest request(*pending_request_.get());
+  // If there was an error navigating to page, return loading failed.
+  if (page_load_state_ != SUCCESS) {
+    Offliner::RequestStatus status =
+        (page_load_state_ == RETRIABLE)
+            ? Offliner::RequestStatus::LOADING_FAILED
+            : Offliner::RequestStatus::LOADING_FAILED_NO_RETRY;
+    completion_callback_.Run(request, status);
+    ResetState();
+    return;
+  }
+
+  save_state_ = SAVING;
+  content::WebContents* web_contents(
+      content::WebContentsObserver::web_contents());
+
+  std::unique_ptr<OfflinePageArchiver> archiver(
+      new OfflinePageMHTMLArchiver(web_contents));
+
+  OfflinePageModel::SavePageParams params;
+  params.url = web_contents->GetLastCommittedURL();
+  params.client_id = request.client_id();
+  params.proposed_offline_id = request.request_id();
+  params.is_background = true;
+
+  // Pass in the original URL if it's different from last committed
+  // when redirects occur.
+  if (params.url != request.url())
+    params.original_url = request.url();
+
+  offline_page_model_->SavePage(
+      params, std::move(archiver),
+      base::Bind(&BackgroundLoaderOffliner::OnPageSaved,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
 void BackgroundLoaderOffliner::OnPageSaved(SavePageResult save_result,
                                            int64_t offline_id) {
   if (!pending_request_)
     return;
 
   SavePageRequest request(*pending_request_.get());
   ResetState();
 
   if (save_state_ == DELETE_AFTER_SAVE) {
     save_state_ = NONE;
@@ skipping 30 matching lines @@
       application_state ==
           base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES) {
     DVLOG(1) << "App became active, canceling current offlining request";
     SavePageRequest* request = pending_request_.get();
     Cancel();
     completion_callback_.Run(*request, RequestStatus::FOREGROUND_CANCELED);
   }
 }
 
 }  // namespace offline_pages