[Offline pages]: Move logic for whether to consider the DocumentAvailableInMainFrame signal to the …

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/offliner_policy.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/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_user_data.h"
 
 namespace offline_pages {
 
 namespace {
-const long kOfflinePageDelayMs = 2000;
-const long kOfflineDomContentLoadedMs = 25000;
 
 class OfflinerData : public content::WebContentsUserData<OfflinerData> {
  public:
   static void AddToWebContents(content::WebContents* webcontents,
                                BackgroundLoaderOffliner* offliner) {
     DCHECK(offliner);
     webcontents->SetUserData(UserDataKey(), std::unique_ptr<OfflinerData>(
                                                 new OfflinerData(offliner)));
   }
 
@@ skipping 32 matching lines @@
 BackgroundLoaderOffliner::BackgroundLoaderOffliner(
     content::BrowserContext* browser_context,
     const OfflinerPolicy* policy,
     OfflinePageModel* offline_page_model)
     : browser_context_(browser_context),
       offline_page_model_(offline_page_model),
       policy_(policy),
       is_low_end_device_(base::SysInfo::IsLowEndDevice()),
       save_state_(NONE),
       page_load_state_(SUCCESS),
-      page_delay_ms_(kOfflinePageDelayMs),
       network_bytes_(0LL),
       weak_ptr_factory_(this) {
   DCHECK(offline_page_model_);
   DCHECK(browser_context_);
 }
 
 BackgroundLoaderOffliner::~BackgroundLoaderOffliner() {}
 
 // static
 BackgroundLoaderOffliner* BackgroundLoaderOffliner::FromWebContents(
@@ skipping 73 matching lines @@
   progress_callback_ = progress_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.
   loader_.get()->LoadPage(request.url());
 
-  snapshot_controller_.reset(
-      new SnapshotController(base::ThreadTaskRunnerHandle::Get(), this,
-                             kOfflineDomContentLoadedMs, page_delay_ms_));
+  snapshot_controller_.reset(SnapshotController::CreateForBackgroundOfflining(
+      base::ThreadTaskRunnerHandle::Get(), this));
 
   return true;
 }
 
 void BackgroundLoaderOffliner::Cancel(const CancelCallback& callback) {
   // TODO(chili): We are not able to cancel a pending
   // OfflinePageModel::SaveSnapshot() operation. We will notify caller that
   // cancel completed once the SavePage operation returns.
   if (!pending_request_) {
     callback.Run(0LL);
@@ skipping 84 matching lines @@
     switch (navigation_handle->GetNetErrorCode()) {
       case net::ERR_INTERNET_DISCONNECTED:
         page_load_state_ = DELAY_RETRY;
         break;
       default:
         page_load_state_ = RETRIABLE;
     }
   }
 }
 
-void BackgroundLoaderOffliner::SetPageDelayForTest(long delay_ms) {
-  page_delay_ms_ = delay_ms;
+void BackgroundLoaderOffliner::SetSnapshotControllerForTest(

[fgorski, 2017/04/21 16:05:10]

+1 one to the use of injection that way! I think this provides for a cleaner
experience, since now you can create the controller in test.

However, because your are taking ownership inside of BLO, you should pass by
unique_ptr<>. Please fix.

This is very good otherwise.

[chili, 2017/04/24 20:34:14]

Done.

+    SnapshotController* controller) {
+  snapshot_controller_.reset(controller);
 }
 
 void BackgroundLoaderOffliner::OnNetworkBytesChanged(int64_t bytes) {
   if (pending_request_ && save_state_ != SAVING) {
     network_bytes_ += bytes;
     progress_callback_.Run(*pending_request_, network_bytes_);
   }
 }
 
 void BackgroundLoaderOffliner::StartSnapshot() {
@@ skipping 121 matching lines @@
     int64_t offline_id) {
   // If for some reason the request was reset during while waiting for callback
   // ignore the completion callback.
   if (pending_request_ && pending_request_->request_id() != offline_id)
     return;
   completion_callback_.Run(request, RequestStatus::FOREGROUND_CANCELED);
 }
 }  // namespace offline_pages
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(offline_pages::OfflinerData);