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

chrome/browser/android/offline_pages/background_loader_offliner.cc

Index: chrome/browser/android/offline_pages/background_loader_offliner.cc
diff --git a/chrome/browser/android/offline_pages/background_loader_offliner.cc b/chrome/browser/android/offline_pages/background_loader_offliner.cc
index 2bf511a66d992509ba0bf92c682e0e22dc883d88..ea41256f640a6f6ad7fec0e214f0e36d5c6f64e1 100644
--- a/chrome/browser/android/offline_pages/background_loader_offliner.cc
+++ b/chrome/browser/android/offline_pages/background_loader_offliner.cc
@@ -18,6 +18,10 @@
 
 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,
@@ -94,6 +98,9 @@ bool BackgroundLoaderOffliner::LoadAndSave(const SavePageRequest& request,
   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;
@@ -129,40 +136,15 @@ void BackgroundLoaderOffliner::DidStopLoading() {
     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;
-  }
-
-  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;
+  // Invalidate ptrs for any ongoing save operation.
+  weak_ptr_factory_.InvalidateWeakPtrs();
 
-  // 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(
@@ -232,6 +214,48 @@ void BackgroundLoaderOffliner::DidFinishNavigation(
   }
 }
 
+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_)