Support serving offline page by offline ID

chrome/browser/android/offline_pages/offline_page_request_job_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/offline_page_request_job.h"
 
 #include "base/callback.h"
 #include "base/feature_list.h"
 #include "base/files/file_path.h"
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/field_trial.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/test/histogram_tester.h"
+#include "base/test/simple_test_clock.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/android/offline_pages/offline_page_model_factory.h"
 #include "chrome/browser/android/offline_pages/offline_page_request_interceptor.h"
 #include "chrome/browser/android/offline_pages/offline_page_tab_helper.h"
 #include "chrome/browser/android/offline_pages/test_offline_page_model_builder.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chrome/test/base/testing_profile_manager.h"
 #include "components/offline_pages/client_namespace_constants.h"
-#include "components/offline_pages/offline_page_model.h"
+#include "components/offline_pages/offline_page_model_impl.h"
 #include "components/previews/previews_experiments.h"
 #include "components/variations/variations_associated_data.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/resource_request_info.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/resource_type.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "net/nqe/network_quality_estimator.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_intercepting_job_factory.h"
 #include "net/url_request/url_request_job_factory_impl.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace offline_pages {
 
 namespace {
 
 const GURL kTestUrl("http://test.org/page1");
 const GURL kTestUrl2("http://test.org/page2");
 const ClientId kTestClientId = ClientId(kBookmarkNamespace, "1234");
+const ClientId kTestClientId2 = ClientId(kDownloadNamespace, "1a2b3c4d");
 const int kTestFileSize = 444;
+const int kTestFileSize2 = 450;
 const int kTabId = 1;
 const int kBufSize = 1024;
 const char kAggregatedRequestResultHistogram[] =
     "OfflinePages.AggregatedRequestResult";
 
 class OfflinePageRequestJobTestDelegate :
     public OfflinePageRequestJob::Delegate {
  public:
   OfflinePageRequestJobTestDelegate(content::WebContents* web_content,
                                     int tab_id)
@@ skipping 159 matching lines @@
  private:
   base::FieldTrialList field_trial_list_;
   std::map<std::string, std::string> network_quality_estimator_params_;
   std::unique_ptr<TestNetworkQualityEstimator> test_network_quality_estimator_;
   net::URLRequestContext* url_request_context_;
 };
 
 class TestOfflinePageArchiver : public OfflinePageArchiver {
  public:
   TestOfflinePageArchiver(const GURL& url,
-                          const base::FilePath& archive_file_path)
+                          const base::FilePath& archive_file_path,
+                          int archive_file_size)
       : url_(url),
-        archive_file_path_(archive_file_path) {}
+        archive_file_path_(archive_file_path),
+        archive_file_size_(archive_file_size) {}
   ~TestOfflinePageArchiver() override {}
 
   void CreateArchive(const base::FilePath& archives_dir,
                      int64_t archive_id,
                      const CreateArchiveCallback& callback) override {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,
         base::Bind(callback, this, ArchiverResult::SUCCESSFULLY_CREATED,
-                   url_, archive_file_path_, base::string16(), kTestFileSize));
+                   url_, archive_file_path_, base::string16(),
+                   archive_file_size_));
   }
 
  private:
   const GURL url_;
   const base::FilePath archive_file_path_;
+  const int archive_file_size_;
 
   DISALLOW_COPY_AND_ASSIGN(TestOfflinePageArchiver);
 };
 
 }  // namespace
 
 class OfflinePageRequestJobTest : public testing::Test {
  public:
   OfflinePageRequestJobTest();
   ~OfflinePageRequestJobTest() override {}
@@ skipping 13 matching lines @@
       OfflinePageRequestJob::AggregatedRequestResult result);
 
   net::TestURLRequestContext* url_request_context() {
     return test_url_request_context_.get();
   }
   Profile* profile() { return profile_; }
   OfflinePageTabHelper* offline_page_tab_helper() const {
     return offline_page_tab_helper_;
   }
   int64_t offline_id() const { return offline_id_; }
+  int64_t offline_id2() const { return offline_id2_; }
   int bytes_read() const { return bytes_read_; }
 
  private:
   void OnSavePageDone(SavePageResult result, int64_t offline_id);
   std::unique_ptr<net::URLRequest> CreateRequest(
       const GURL& url,
       const std::string& method,
       content::ResourceType resource_type);
   void ReadCompleted(int bytes_read);
 
@@ skipping 14 matching lines @@
       intercepting_job_factory_;
   std::unique_ptr<TestURLRequestDelegate> url_request_delegate_;
   net::TestNetworkDelegate network_delegate_;
   TestingProfileManager profile_manager_;
   TestingProfile* profile_;
   std::unique_ptr<content::WebContents> web_contents_;
   base::HistogramTester histogram_tester_;
   OfflinePageTabHelper* offline_page_tab_helper_;  // Not owned.
   std::unique_ptr<net::URLRequest> request_;
   int64_t offline_id_;
+  int64_t offline_id2_;
   int bytes_read_;
 
   DISALLOW_COPY_AND_ASSIGN(OfflinePageRequestJobTest);
 };
 
 OfflinePageRequestJobTest::OfflinePageRequestJobTest()
     : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP),
       network_change_notifier_(new TestNetworkChangeNotifier()),
       profile_manager_(TestingBrowserProcess::GetGlobal()),
       offline_id_(-1),
+      offline_id2_(-1),
       bytes_read_(0)  {
 }
 
 void OfflinePageRequestJobTest::SetUp() {
   // Create a test profile.
   ASSERT_TRUE(profile_manager_.SetUp());
   profile_ = profile_manager_.CreateTestingProfile("Profile 1");
 
   // Create a test web contents.
   web_contents_.reset(content::WebContents::Create(
       content::WebContents::CreateParams(profile_)));
   OfflinePageTabHelper::CreateForWebContents(web_contents_.get());
   offline_page_tab_helper_ =
       OfflinePageTabHelper::FromWebContents(web_contents_.get());
 
   // Set up the factory for testing.
   OfflinePageModelFactory::GetInstance()->SetTestingFactoryAndUse(
       profile(), BuildTestOfflinePageModel);
   RunUntilIdle();
 
-  // Use a test archive file.
-  base::FilePath archive_file_path;
-  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &archive_file_path));
-  archive_file_path =
-      archive_file_path.AppendASCII("offline_pages").AppendASCII("test.mhtml");
-  std::unique_ptr<TestOfflinePageArchiver> archiver(
-      new TestOfflinePageArchiver(kTestUrl, archive_file_path));
+  OfflinePageModel* model =
+      OfflinePageModelFactory::GetForBrowserContext(profile());
+
+  // Hook up a test clock such that we can control the time when the offline
+  // page is created.
+  std::unique_ptr<base::SimpleTestClock> clock(new base::SimpleTestClock());
+  base::SimpleTestClock* temp_clock = clock.get();
+  clock->SetNow(base::Time::Now());
+  static_cast<OfflinePageModelImpl*>(model)->SetClockForTesting(
+      std::move(clock));

[Dmitry Titov, 2016/09/09 04:33:06]

This is very unorthodox thing you are doing here with capturing a raw pointer
and then std::move'ing it out... I don't think this is how the std::unique_ptr's
are used :)

Correct solutions exist but they are a bit more involved, which is perhaps why
you went this route. I'd rather avoid doing the whole thing with test clock in
this potentially mergeable CL, especially considering the test is only
marginally improved by it. 

[jianli, 2016/09/10 00:56:04]

Updated per discussion.

+
+  // All offline pages being created below will point to real archive files
+  // residing in test data directory.
+  base::FilePath test_data_dir_path;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir_path));
 
   // Save an offline page.
-  OfflinePageModel* model =
-      OfflinePageModelFactory::GetForBrowserContext(profile());
+  base::FilePath archive_file_path =
+      test_data_dir_path.AppendASCII("offline_pages").AppendASCII("test.mhtml");
+  std::unique_ptr<TestOfflinePageArchiver> archiver(
+      new TestOfflinePageArchiver(kTestUrl, archive_file_path, kTestFileSize));
+
   model->SavePage(
       kTestUrl, kTestClientId, 0, std::move(archiver),
       base::Bind(&OfflinePageRequestJobTest::OnSavePageDone,
                 base::Unretained(this)));
   RunUntilIdle();
 
+  // Save another offline page associated with same online URL as above, but
+  // pointing to different archive file.
+  base::FilePath archive_file_path2 =
+      test_data_dir_path.AppendASCII("offline_pages").
+          AppendASCII("hello.mhtml");
+  std::unique_ptr<TestOfflinePageArchiver> archiver2(
+      new TestOfflinePageArchiver(
+          kTestUrl, archive_file_path2, kTestFileSize2));
+
+  // Make sure that the creation time of 2nd offline file is later.
+  temp_clock->Advance(base::TimeDelta::FromMinutes(10));
+
+  model->SavePage(
+      kTestUrl, kTestClientId2, 0, std::move(archiver2),
+      base::Bind(&OfflinePageRequestJobTest::OnSavePageDone,
+                base::Unretained(this)));
+  RunUntilIdle();
+
   // Create a context with delayed initialization.
   test_url_request_context_.reset(new net::TestURLRequestContext(true));
 
   // Install the interceptor.
   std::unique_ptr<net::URLRequestInterceptor> interceptor(
       new OfflinePageRequestInterceptor(profile_));
   std::unique_ptr<net::URLRequestJobFactoryImpl> job_factory_impl(
       new net::URLRequestJobFactoryImpl());
   intercepting_job_factory_.reset(new TestURLRequestInterceptingJobFactory(
       std::move(job_factory_impl),
@@ skipping 42 matching lines @@
   return request;
 }
 
 void OfflinePageRequestJobTest::ExpectAggregatedRequestResultHistogram(
     OfflinePageRequestJob::AggregatedRequestResult result) {
   histogram_tester_.ExpectUniqueSample(
       kAggregatedRequestResultHistogram, static_cast<int>(result), 1);
 }
 
 void OfflinePageRequestJobTest::OnSavePageDone(SavePageResult result,
-                                                   int64_t offline_id) {
+                                               int64_t offline_id) {
   ASSERT_EQ(SavePageResult::SUCCESS, result);
-  offline_id_ = offline_id;
+  if (offline_id_ == -1)
+    offline_id_ = offline_id;
+  else if (offline_id2_ == -1)
+    offline_id2_ = offline_id;
 }
 
 void OfflinePageRequestJobTest::InterceptRequestOnIO(
     const GURL& url,
     const std::string& method,
     const std::string& extra_header_name,
     const std::string& extra_header_value,
     content::ResourceType resource_type,
     void* profile_id) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
@@ skipping 80 matching lines @@
   EXPECT_EQ(0, bytes_read());
   EXPECT_FALSE(offline_page_tab_helper()->offline_page());
 }
 
 TEST_F(OfflinePageRequestJobTest, LoadOfflinePageOnDisconnectedNetwork) {
   SimulateHasNetworkConnectivity(false);
 
   InterceptRequest(kTestUrl, "GET", "", "", content::RESOURCE_TYPE_MAIN_FRAME);
   base::RunLoop().Run();
 
-  EXPECT_EQ(kTestFileSize, bytes_read());
+  EXPECT_EQ(kTestFileSize2, bytes_read());
   ASSERT_TRUE(offline_page_tab_helper()->offline_page());
-  EXPECT_EQ(offline_id(),
+  EXPECT_EQ(offline_id2(),
             offline_page_tab_helper()->offline_page()->offline_id);
   ExpectAggregatedRequestResultHistogram(
       OfflinePageRequestJob::AggregatedRequestResult::
           SHOW_OFFLINE_ON_DISCONNECTED_NETWORK);
 }
 
 TEST_F(OfflinePageRequestJobTest, PageNotFoundOnDisconnectedNetwork) {
   SimulateHasNetworkConnectivity(false);
 
   InterceptRequest(kTestUrl2, "GET", "", "", content::RESOURCE_TYPE_MAIN_FRAME);
   base::RunLoop().Run();
 
   EXPECT_EQ(0, bytes_read());
   EXPECT_FALSE(offline_page_tab_helper()->offline_page());
   ExpectAggregatedRequestResultHistogram(
       OfflinePageRequestJob::AggregatedRequestResult::
           PAGE_NOT_FOUND_ON_DISCONNECTED_NETWORK);
 }
 
 TEST_F(OfflinePageRequestJobTest, LoadOfflinePageOnProhibitivelySlowNetwork) {
   SimulateHasNetworkConnectivity(true);
 
   ScopedEnableProbihibitivelySlowNetwork scoped(url_request_context());
 
   InterceptRequest(kTestUrl, "GET", "", "", content::RESOURCE_TYPE_MAIN_FRAME);
   base::RunLoop().Run();
 
-  EXPECT_EQ(kTestFileSize, bytes_read());
+  EXPECT_EQ(kTestFileSize2, bytes_read());
   ASSERT_TRUE(offline_page_tab_helper()->offline_page());
-  EXPECT_EQ(offline_id(),
+  EXPECT_EQ(offline_id2(),
             offline_page_tab_helper()->offline_page()->offline_id);
   ExpectAggregatedRequestResultHistogram(
       OfflinePageRequestJob::AggregatedRequestResult::
           SHOW_OFFLINE_ON_PROHIBITIVELY_SLOW_NETWORK);
 }
 
 TEST_F(OfflinePageRequestJobTest, PageNotFoundOnProhibitivelySlowNetwork) {
   SimulateHasNetworkConnectivity(true);
 
   ScopedEnableProbihibitivelySlowNetwork scoped(url_request_context());
 
   InterceptRequest(kTestUrl2, "GET", "", "", content::RESOURCE_TYPE_MAIN_FRAME);
   base::RunLoop().Run();
 
   EXPECT_EQ(0, bytes_read());
   EXPECT_FALSE(offline_page_tab_helper()->offline_page());
   ExpectAggregatedRequestResultHistogram(
       OfflinePageRequestJob::AggregatedRequestResult::
           PAGE_NOT_FOUND_ON_PROHIBITIVELY_SLOW_NETWORK);
 }
 
 TEST_F(OfflinePageRequestJobTest, LoadOfflinePageOnFlakyNetwork) {
   SimulateHasNetworkConnectivity(true);
 
   // When custom offline header exists and contains "reason=error", it means
   // that net error is hit in last request due to flaky network.
   InterceptRequest(
       kTestUrl,
       "GET",
-      kLoadingOfflinePageHeader,
-      std::string(kLoadingOfflinePageReason) + kLoadingOfflinePageDueToNetError,
+      kOfflinePageHeader,
+      std::string(kOfflinePageHeaderReasonKey) + "=" +
+          kOfflinePageHeaderReasonValueDueToNetError,
       content::RESOURCE_TYPE_MAIN_FRAME);
   base::RunLoop().Run();
 
-  EXPECT_EQ(kTestFileSize, bytes_read());
+  EXPECT_EQ(kTestFileSize2, bytes_read());
   ASSERT_TRUE(offline_page_tab_helper()->offline_page());
-  EXPECT_EQ(offline_id(),
+  EXPECT_EQ(offline_id2(),
             offline_page_tab_helper()->offline_page()->offline_id);
   ExpectAggregatedRequestResultHistogram(
       OfflinePageRequestJob::AggregatedRequestResult::
           SHOW_OFFLINE_ON_FLAKY_NETWORK);
 }
 
 TEST_F(OfflinePageRequestJobTest, PageNotFoundOnFlakyNetwork) {
   SimulateHasNetworkConnectivity(true);
 
   // When custom offline header exists and contains "reason=error", it means
   // that net error is hit in last request due to flaky network.
   InterceptRequest(
       kTestUrl2,
       "GET",
-      kLoadingOfflinePageHeader,
-      std::string(kLoadingOfflinePageReason) + kLoadingOfflinePageDueToNetError,
+      kOfflinePageHeader,
+      std::string(kOfflinePageHeaderReasonKey) + "=" +
+          kOfflinePageHeaderReasonValueDueToNetError,
       content::RESOURCE_TYPE_MAIN_FRAME);
   base::RunLoop().Run();
 
   EXPECT_EQ(0, bytes_read());
   EXPECT_FALSE(offline_page_tab_helper()->offline_page());
   ExpectAggregatedRequestResultHistogram(
       OfflinePageRequestJob::AggregatedRequestResult::
           PAGE_NOT_FOUND_ON_FLAKY_NETWORK);
 }
 
 TEST_F(OfflinePageRequestJobTest, ForceLoadOfflinePageOnConnectedNetwork) {
   SimulateHasNetworkConnectivity(true);
 
   // When custom offline header exists and contains value other than
   // "reason=error", it means that offline page is forced to load.
   InterceptRequest(
       kTestUrl,
       "GET",
-      kLoadingOfflinePageHeader,
-      std::string(kLoadingOfflinePageReason) + "download",
+      kOfflinePageHeader,
+      std::string(kOfflinePageHeaderReasonKey) + "=download",
       content::RESOURCE_TYPE_MAIN_FRAME);
   base::RunLoop().Run();
 
-  EXPECT_EQ(kTestFileSize, bytes_read());
+  EXPECT_EQ(kTestFileSize2, bytes_read());
   ASSERT_TRUE(offline_page_tab_helper()->offline_page());
-  EXPECT_EQ(offline_id(),
+  EXPECT_EQ(offline_id2(),
             offline_page_tab_helper()->offline_page()->offline_id);
   ExpectAggregatedRequestResultHistogram(
       OfflinePageRequestJob::AggregatedRequestResult::
           SHOW_OFFLINE_ON_CONNECTED_NETWORK);
 }
 
 TEST_F(OfflinePageRequestJobTest, PageNotFoundOnConnectedNetwork) {
   SimulateHasNetworkConnectivity(true);
 
   // When custom offline header exists and contains value other than
   // "reason=error", it means that offline page is forced to load.
   InterceptRequest(
       kTestUrl2,
       "GET",
-      kLoadingOfflinePageHeader,
-      std::string(kLoadingOfflinePageReason) + "download",
+      kOfflinePageHeader,
+      std::string(kOfflinePageHeaderReasonKey) + "=download",
       content::RESOURCE_TYPE_MAIN_FRAME);
   base::RunLoop().Run();
 
   EXPECT_EQ(0, bytes_read());
   EXPECT_FALSE(offline_page_tab_helper()->offline_page());
   ExpectAggregatedRequestResultHistogram(
       OfflinePageRequestJob::AggregatedRequestResult::
           PAGE_NOT_FOUND_ON_CONNECTED_NETWORK);
 }
 
 TEST_F(OfflinePageRequestJobTest, DoNotLoadOfflinePageOnConnectedNetwork) {
   SimulateHasNetworkConnectivity(true);
 
   InterceptRequest(kTestUrl, "GET", "", "", content::RESOURCE_TYPE_MAIN_FRAME);
   base::RunLoop().Run();
 
   EXPECT_EQ(0, bytes_read());
   EXPECT_FALSE(offline_page_tab_helper()->offline_page());
 }
 
+TEST_F(OfflinePageRequestJobTest, LoadOfflinePageByClientID) {
+  SimulateHasNetworkConnectivity(true);
+
+  InterceptRequest(
+      kTestUrl,
+      "GET",
+      kOfflinePageHeader,
+      std::string(kOfflinePageHeaderReasonKey) + "=download " +
+          kOfflinePageHeaderNamespaceKey + "=" + kTestClientId.name_space +
+          " " + kOfflinePageHeaderIDKey + "=" + kTestClientId.id,
+      content::RESOURCE_TYPE_MAIN_FRAME);
+  base::RunLoop().Run();
+
+  EXPECT_EQ(kTestFileSize, bytes_read());
+  ASSERT_TRUE(offline_page_tab_helper()->offline_page());
+  EXPECT_EQ(offline_id(),
+            offline_page_tab_helper()->offline_page()->offline_id);
+  ExpectAggregatedRequestResultHistogram(
+      OfflinePageRequestJob::AggregatedRequestResult::
+          SHOW_OFFLINE_ON_CONNECTED_NETWORK);
+}
+
+TEST_F(OfflinePageRequestJobTest,
+       LoadOfflinePageByClientIDAndFallbackToOnlineURL) {
+  SimulateHasNetworkConnectivity(true);
+
+  // The offline page found with specific client ID does not match the passed
+  // online URL. Should fall back to find the offline page based on the online
+  // URL.
+  InterceptRequest(
+      kTestUrl2,
+      "GET",
+      kOfflinePageHeader,
+      std::string(kOfflinePageHeaderReasonKey) + "=download " +
+          kOfflinePageHeaderNamespaceKey + "=" + kTestClientId.name_space +
+          " " + kOfflinePageHeaderIDKey + "=" + kTestClientId.id,
+      content::RESOURCE_TYPE_MAIN_FRAME);
+  base::RunLoop().Run();
+
+  EXPECT_EQ(0, bytes_read());
+  EXPECT_FALSE(offline_page_tab_helper()->offline_page());
+  ExpectAggregatedRequestResultHistogram(
+      OfflinePageRequestJob::AggregatedRequestResult::
+          PAGE_NOT_FOUND_ON_CONNECTED_NETWORK);
+}
+
+TEST_F(OfflinePageRequestJobTest, LoadOfflinePageByOfflineID) {
+  SimulateHasNetworkConnectivity(true);
+
+  InterceptRequest(
+      kTestUrl,
+      "GET",
+      kOfflinePageHeader,
+      std::string(kOfflinePageHeaderReasonKey) + "=download " +
+          kOfflinePageHeaderIDKey + "=" + base::Int64ToString(offline_id()),
+      content::RESOURCE_TYPE_MAIN_FRAME);
+  base::RunLoop().Run();
+
+  EXPECT_EQ(kTestFileSize, bytes_read());
+  ASSERT_TRUE(offline_page_tab_helper()->offline_page());
+  EXPECT_EQ(offline_id(),
+            offline_page_tab_helper()->offline_page()->offline_id);
+  ExpectAggregatedRequestResultHistogram(
+      OfflinePageRequestJob::AggregatedRequestResult::
+          SHOW_OFFLINE_ON_CONNECTED_NETWORK);
+}
+
+TEST_F(OfflinePageRequestJobTest,
+       LoadOfflinePageByOfflineIDAndFallbackToOnlineURL) {
+  SimulateHasNetworkConnectivity(true);
+
+  // The offline page found with specific offline ID does not match the passed
+  // online URL. Should fall back to find the offline page based on the online
+  // URL.
+  InterceptRequest(
+      kTestUrl2,
+      "GET",
+      kOfflinePageHeader,
+      std::string(kOfflinePageHeaderReasonKey) + "=download " +
+          kOfflinePageHeaderIDKey + "=" + base::Int64ToString(offline_id()),
+      content::RESOURCE_TYPE_MAIN_FRAME);
+  base::RunLoop().Run();
+
+  EXPECT_EQ(0, bytes_read());
+  EXPECT_FALSE(offline_page_tab_helper()->offline_page());
+  ExpectAggregatedRequestResultHistogram(
+      OfflinePageRequestJob::AggregatedRequestResult::
+          PAGE_NOT_FOUND_ON_CONNECTED_NETWORK);
+}
+
 }  // namespace offline_pages