[Offline pages]: Implement background loader to save on last retry, and record last retry success U…

chrome/browser/android/offline_pages/background_loader_offliner_unittest.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/bind.h"
 #include "base/run_loop.h"
 #include "base/test/histogram_tester.h"
 #include "base/test/scoped_mock_time_message_loop_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/android/offline_pages/offliner_helper.h"
 #include "chrome/browser/net/prediction_options.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/content_settings/core/common/pref_names.h"
 #include "components/offline_pages/content/background_loader/background_loader_contents_stub.h"
 #include "components/offline_pages/core/background/offliner.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/stub_offline_page_model.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "content/public/test/web_contents_tester.h"
 #include "net/base/net_errors.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ skipping 110 matching lines @@
                       base::Unretained(this));
   }
   Profile* profile() { return &profile_; }
   bool completion_callback_called() { return completion_callback_called_; }
   Offliner::RequestStatus request_status() { return request_status_; }
   bool cancel_callback_called() { return cancel_callback_called_; }
   bool SaveInProgress() const { return model_->mock_saving(); }
   MockOfflinePageModel* model() const { return model_; }
   const base::HistogramTester& histograms() const { return histogram_tester_; }
   int64_t progress() { return progress_; }
+  OfflinerPolicy* policy() const { return policy_.get(); }
 
   void PumpLoop() { base::RunLoop().RunUntilIdle(); }
 
   void CompleteLoading() {
     offliner()->DocumentOnLoadCompletedInMainFrame();
     PumpLoop();
   }
 
  private:
   void OnCompletion(const SavePageRequest& request,
                     Offliner::RequestStatus status);
   void OnProgress(const SavePageRequest& request, int64_t bytes);
   void OnCancel(int64_t offline_id);
   content::TestBrowserThreadBundle thread_bundle_;
   TestingProfile profile_;
+  std::unique_ptr<OfflinerPolicy> policy_;
   std::unique_ptr<TestBackgroundLoaderOffliner> offliner_;
   MockOfflinePageModel* model_;
   bool completion_callback_called_;
   bool cancel_callback_called_;
   int64_t progress_;
   Offliner::RequestStatus request_status_;
   base::HistogramTester histogram_tester_;
 
   DISALLOW_COPY_AND_ASSIGN(BackgroundLoaderOfflinerTest);
 };
 
 BackgroundLoaderOfflinerTest::BackgroundLoaderOfflinerTest()
     : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP),
       completion_callback_called_(false),
       cancel_callback_called_(false),
       progress_(0LL),
       request_status_(Offliner::RequestStatus::UNKNOWN) {}
 
 BackgroundLoaderOfflinerTest::~BackgroundLoaderOfflinerTest() {}
 
 void BackgroundLoaderOfflinerTest::SetUp() {
   model_ = new MockOfflinePageModel();
-  offliner_.reset(new TestBackgroundLoaderOffliner(profile(), nullptr, model_));
+  policy_.reset(new OfflinerPolicy());
+  offliner_.reset(
+      new TestBackgroundLoaderOffliner(profile(), policy_.get(), model_));
   offliner_->SetPageDelayForTest(0L);
 }
 
 void BackgroundLoaderOfflinerTest::OnCompletion(
     const SavePageRequest& request,
     Offliner::RequestStatus status) {
   DCHECK(!completion_callback_called_);  // Expect 1 callback per request.
   completion_callback_called_ = true;
   request_status_ = status;
 }
@@ skipping 279 matching lines @@
   PumpLoop();
   model()->CompleteSavingAsSuccess();
   PumpLoop();
 
   EXPECT_TRUE(completion_callback_called());
   EXPECT_EQ(Offliner::RequestStatus::SAVED, request_status());
   EXPECT_FALSE(offliner()->is_loading());
   EXPECT_FALSE(SaveInProgress());
 }
 
+TEST_F(BackgroundLoaderOfflinerTest, HandleTimeoutWithLowBarStartedTriesMet) {

[Pete Williamson, 2017/04/13 22:57:15]

Should we have a test where we get a last timeout, but the low bar is not met?

[chili, 2017/04/15 00:34:59]

That is the situation in HandleTimeoutWithStartedTriesMet

+  base::Time creation_time = base::Time::Now();
+  SavePageRequest request(kRequestId, kHttpUrl, kClientId, creation_time,
+                          kUserRequested);
+  EXPECT_TRUE(offliner()->LoadAndSave(request, completion_callback(),
+                                      progress_callback()));
+  request.set_started_attempt_count(policy()->GetMaxStartedTries() - 1);
+  // Sets lowbar.
+  offliner()->DocumentAvailableInMainFrame();
+  // Timeout
+  EXPECT_TRUE(offliner()->HandleTimeout(request));
+  EXPECT_TRUE(SaveInProgress());
+  model()->CompleteSavingAsSuccess();
+  PumpLoop();
+  EXPECT_EQ(Offliner::RequestStatus::SAVED_ON_LAST_RETRY, request_status());
+}
+
+TEST_F(BackgroundLoaderOfflinerTest, HandleTimeoutWithStartedTriesMet) {

[Pete Williamson, 2017/04/13 22:57:15]

Should we test both started and finished attempt count?

[romax, 2017/04/13 23:17:14]

I think the test is missing for the prerendering_offliner part as well (it was
my bad :( , can you please give it a quick fix if possible?

[chili, 2017/04/15 00:34:59]

Done.

+  base::Time creation_time = base::Time::Now();
+  SavePageRequest request(kRequestId, kHttpUrl, kClientId, creation_time,
+                          kUserRequested);
+  EXPECT_TRUE(offliner()->LoadAndSave(request, completion_callback(),
+                                      progress_callback()));
+  request.set_started_attempt_count(policy()->GetMaxStartedTries() - 1);
+  // Timeout
+  EXPECT_FALSE(offliner()->HandleTimeout(request));
+  EXPECT_FALSE(SaveInProgress());
+}
+
+TEST_F(BackgroundLoaderOfflinerTest, HandleTimeoutWithLowBar) {

[Pete Williamson, 2017/04/13 22:57:15]

Nit - this test name might be better as 
HandleTimeoutWithLowBarButNotAtRetryLimit

or something like that.

+  base::Time creation_time = base::Time::Now();
+  SavePageRequest request(kRequestId, kHttpUrl, kClientId, creation_time,
+                          kUserRequested);
+  EXPECT_TRUE(offliner()->LoadAndSave(request, completion_callback(),
+                                      progress_callback()));
+  // Timeout
+  EXPECT_FALSE(offliner()->HandleTimeout(request));
+  EXPECT_FALSE(SaveInProgress());
+}
+
 }  // namespace offline_pages