[Offline Pages] PF: Checks Chrome settings for prefetching being enabled

Adds a new PrefetchConfiguration component to PrefethService that serves
as an interface to the embedder to obtain user settings information.
Initially it allows determining if the prefetching of offline pages
should be enabled based on current user settings.

Bug: 701939
Change-Id: Ia33e1264c11acce2a09cd8585f5b484c6a750182
Reviewed-on: https://chromium-review.googlesource.com/618447
Commit-Queue: Justin DeWitt <dewittj@chromium.org>
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Reviewed-by: Dmitry Titov <dimich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495574}

chrome/browser/BUILD.gn

@@ -2236,6 +2236,8 @@ split_static_library("browser") {
       "offline_pages/prefetch/prefetch_background_task.h",
       "offline_pages/prefetch/prefetch_background_task_handler_impl.cc",
       "offline_pages/prefetch/prefetch_background_task_handler_impl.h",
+      "offline_pages/prefetch/prefetch_configuration_impl.cc",
+      "offline_pages/prefetch/prefetch_configuration_impl.h",
       "offline_pages/prefetch/prefetch_importer_impl.cc",
       "offline_pages/prefetch/prefetch_importer_impl.h",
       "offline_pages/prefetch/prefetch_instance_id_proxy.cc",

chrome/browser/offline_pages/prefetch/prefetch_configuration_impl.cc

@@ -0,0 +1,23 @@
+// Copyright 2017 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/offline_pages/prefetch/prefetch_configuration_impl.h"
+
+#include "chrome/browser/net/prediction_options.h"
+#include "chrome/common/pref_names.h"
+#include "components/prefs/pref_service.h"
+
+namespace offline_pages {
+
+PrefetchConfigurationImpl::PrefetchConfigurationImpl(PrefService* pref_service)
+    : pref_service_(pref_service) {}
+
+PrefetchConfigurationImpl::~PrefetchConfigurationImpl() = default;
+
+bool PrefetchConfigurationImpl::IsPrefetchingEnabledBySettings() {
+  return pref_service_->GetInteger(prefs::kNetworkPredictionOptions) !=
+         chrome_browser_net::NETWORK_PREDICTION_NEVER;
+}
+
+}  // namespace offline_pages

chrome/browser/offline_pages/prefetch/prefetch_configuration_impl.h

@@ -0,0 +1,32 @@
+// Copyright 2017 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.
+
+#ifndef CHROME_BROWSER_OFFLINE_PAGES_PREFETCH_PREFETCH_CONFIGURATION_IMPL_H_
+#define CHROME_BROWSER_OFFLINE_PAGES_PREFETCH_PREFETCH_CONFIGURATION_IMPL_H_
+
+#include "base/macros.h"
+#include "components/offline_pages/core/prefetch/prefetch_configuration.h"
+
+class PrefService;
+
+namespace offline_pages {
+
+// Implementation of PrefetchConfiguration that queries Chrome preferences.
+class PrefetchConfigurationImpl : public PrefetchConfiguration {
+ public:
+  explicit PrefetchConfigurationImpl(PrefService* pref_service);
+  ~PrefetchConfigurationImpl() override;
+
+  // PrefetchConfiguration implementation.
+  bool IsPrefetchingEnabledBySettings() override;
+
+ private:
+  PrefService* pref_service_;
+
+  DISALLOW_COPY_AND_ASSIGN(PrefetchConfigurationImpl);
+};
+
+}  // namespace offline_pages
+
+#endif  // CHROME_BROWSER_OFFLINE_PAGES_PREFETCH_PREFETCH_CONFIGURATION_IMPL_H_

chrome/browser/offline_pages/prefetch/prefetch_service_factory.cc

@@ -12,10 +12,10 @@
 #include "base/memory/singleton.h"
 #include "base/sequenced_task_runner.h"
 #include "base/task_scheduler/post_task.h"
-#include "build/build_config.h"
 #include "chrome/browser/download/download_service_factory.h"
 #include "chrome/browser/offline_pages/prefetch/offline_metrics_collector_impl.h"
 #include "chrome/browser/offline_pages/prefetch/prefetch_background_task_handler_impl.h"
+#include "chrome/browser/offline_pages/prefetch/prefetch_configuration_impl.h"
 #include "chrome/browser/offline_pages/prefetch/prefetch_importer_impl.h"
 #include "chrome/browser/offline_pages/prefetch/prefetch_instance_id_proxy.h"
 #include "chrome/browser/profiles/profile.h"
@@ -88,18 +88,19 @@ KeyedService* PrefetchServiceFactory::BuildServiceInstanceFor(
   auto prefetch_importer = base::MakeUnique<PrefetchImporterImpl>(
       prefetch_dispatcher.get(), context, background_task_runner);
 
-  std::unique_ptr<PrefetchBackgroundTaskHandler>
-      prefetch_background_task_handler =
-          base::MakeUnique<PrefetchBackgroundTaskHandlerImpl>(
-              profile->GetPrefs());
+  auto prefetch_background_task_handler =
+      base::MakeUnique<PrefetchBackgroundTaskHandlerImpl>(profile->GetPrefs());
+
+  auto configuration =
+      base::MakeUnique<PrefetchConfigurationImpl>(profile->GetPrefs());
 
   return new PrefetchServiceImpl(
       std::move(offline_metrics_collector), std::move(prefetch_dispatcher),
       std::move(prefetch_gcm_app_handler),
       std::move(prefetch_network_request_factory), std::move(prefetch_store),
       std::move(suggested_articles_observer), std::move(prefetch_downloader),
-      std::move(prefetch_importer),
-      std::move(prefetch_background_task_handler));
+      std::move(prefetch_importer), std::move(prefetch_background_task_handler),
+      std::move(configuration));
 }
 
 }  // namespace offline_pages

components/offline_pages/core/prefetch/BUILD.gn

@@ -33,6 +33,8 @@ static_library("prefetch") {
     "page_bundle_update_task.cc",
     "page_bundle_update_task.h",
     "prefetch_background_task_handler.h",
+    "prefetch_configuration.cc",
+    "prefetch_configuration.h",
     "prefetch_dispatcher.h",
     "prefetch_dispatcher_impl.cc",
     "prefetch_dispatcher_impl.h",

components/offline_pages/core/prefetch/prefetch_configuration.cc

@@ -0,0 +1,15 @@
+// Copyright 2017 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 "components/offline_pages/core/prefetch/prefetch_configuration.h"
+
+#include "components/offline_pages/core/offline_page_feature.h"
+
+namespace offline_pages {
+
+bool PrefetchConfiguration::IsPrefetchingEnabled() {
+  return IsPrefetchingOfflinePagesEnabled() && IsPrefetchingEnabledBySettings();
+}
+
+}  // namespace offline_pages

components/offline_pages/core/prefetch/prefetch_configuration.h

@@ -0,0 +1,29 @@
+// Copyright 2017 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.
+
+#ifndef COMPONENTS_OFFLINE_PAGES_CORE_PREFETCH_PREFETCH_CONFIGURATION_H_
+#define COMPONENTS_OFFLINE_PAGES_CORE_PREFETCH_PREFETCH_CONFIGURATION_H_
+
+namespace offline_pages {
+
+// Class that interfaces with configuration systems to provide prefetching
+// specific configuration information.
+class PrefetchConfiguration {
+ public:
+  virtual ~PrefetchConfiguration() = default;
+
+  // Returns true if all needed flags and settings allows the prefetching of
+  // offline pages to run. Note that this result can change in the course of the
+  // application lifetime. Should not be overridden by subclasses.
+  bool IsPrefetchingEnabled();
+
+ protected:
+  // Returns true if user settings allow the prefetching of offline pages to
+  // run. Should not be called by users of this class.
+  virtual bool IsPrefetchingEnabledBySettings() = 0;
+};
+
+}  // namespace offline_pages
+
+#endif  // COMPONENTS_OFFLINE_PAGES_CORE_PREFETCH_PREFETCH_CONFIGURATION_H_

components/offline_pages/core/prefetch/prefetch_dispatcher_impl.cc

@@ -13,7 +13,6 @@
 #include "base/task_scheduler/post_task.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/offline_pages/core/offline_event_logger.h"
-#include "components/offline_pages/core/offline_page_feature.h"
 #include "components/offline_pages/core/prefetch/add_unique_urls_task.h"
 #include "components/offline_pages/core/prefetch/download_archives_task.h"
 #include "components/offline_pages/core/prefetch/download_completed_task.h"
@@ -24,6 +23,7 @@
 #include "components/offline_pages/core/prefetch/mark_operation_done_task.h"
 #include "components/offline_pages/core/prefetch/page_bundle_update_task.h"
 #include "components/offline_pages/core/prefetch/prefetch_background_task_handler.h"
+#include "components/offline_pages/core/prefetch/prefetch_configuration.h"
 #include "components/offline_pages/core/prefetch/prefetch_gcm_handler.h"
 #include "components/offline_pages/core/prefetch/prefetch_importer.h"
 #include "components/offline_pages/core/prefetch/prefetch_network_request_factory.h"
@@ -60,7 +60,7 @@ void PrefetchDispatcherImpl::SchedulePipelineProcessing() {
 void PrefetchDispatcherImpl::AddCandidatePrefetchURLs(
     const std::string& name_space,
     const std::vector<PrefetchURL>& prefetch_urls) {
-  if (!IsPrefetchingOfflinePagesEnabled())
+  if (!service_->GetPrefetchConfiguration()->IsPrefetchingEnabled())
     return;
 
   PrefetchStore* prefetch_store = service_->GetPrefetchStore();
@@ -74,23 +74,23 @@ void PrefetchDispatcherImpl::AddCandidatePrefetchURLs(
 
 void PrefetchDispatcherImpl::RemoveAllUnprocessedPrefetchURLs(
     const std::string& name_space) {
-  if (!IsPrefetchingOfflinePagesEnabled())
+  if (!service_->GetPrefetchConfiguration()->IsPrefetchingEnabled())
     return;
 
   NOTIMPLEMENTED();
 }
 
 void PrefetchDispatcherImpl::RemovePrefetchURLsByClientId(
     const ClientId& client_id) {
-  if (!IsPrefetchingOfflinePagesEnabled())
+  if (!service_->GetPrefetchConfiguration()->IsPrefetchingEnabled())
     return;
 
   NOTIMPLEMENTED();
 }
 
 void PrefetchDispatcherImpl::BeginBackgroundTask(
     std::unique_ptr<ScopedBackgroundTask> background_task) {
-  if (!IsPrefetchingOfflinePagesEnabled())
+  if (!service_->GetPrefetchConfiguration()->IsPrefetchingEnabled())
     return;
   service_->GetLogger()->RecordActivity("Beginning Background Task.");
 
@@ -139,14 +139,14 @@ void PrefetchDispatcherImpl::QueueActionTasks() {
 }
 
 void PrefetchDispatcherImpl::StopBackgroundTask() {
-  if (!IsPrefetchingOfflinePagesEnabled())
+  if (!service_->GetPrefetchConfiguration()->IsPrefetchingEnabled())
     return;
 
   DisposeTask();
 }
 
 void PrefetchDispatcherImpl::RequestFinishBackgroundTaskForTest() {
-  if (!IsPrefetchingOfflinePagesEnabled())
+  if (!service_->GetPrefetchConfiguration()->IsPrefetchingEnabled())
     return;
 
   DisposeTask();
@@ -176,7 +176,7 @@ void PrefetchDispatcherImpl::DisposeTask() {
 
 void PrefetchDispatcherImpl::GCMOperationCompletedMessageReceived(
     const std::string& operation_name) {
-  if (!IsPrefetchingOfflinePagesEnabled())
+  if (!service_->GetPrefetchConfiguration()->IsPrefetchingEnabled())
     return;
 
   PrefetchStore* prefetch_store = service_->GetPrefetchStore();
@@ -206,7 +206,7 @@ void PrefetchDispatcherImpl::DidGetOperationRequest(
 
 void PrefetchDispatcherImpl::DownloadCompleted(
     const PrefetchDownloadResult& download_result) {
-  if (!IsPrefetchingOfflinePagesEnabled())
+  if (!service_->GetPrefetchConfiguration()->IsPrefetchingEnabled())
     return;
 
   service_->GetLogger()->RecordActivity(
@@ -223,7 +223,7 @@ void PrefetchDispatcherImpl::DownloadCompleted(
 }
 
 void PrefetchDispatcherImpl::ImportCompleted(int64_t offline_id, bool success) {
-  if (!IsPrefetchingOfflinePagesEnabled())
+  if (!service_->GetPrefetchConfiguration()->IsPrefetchingEnabled())
     return;
 
   service_->GetLogger()->RecordActivity("Importing archive " +

components/offline_pages/core/prefetch/prefetch_dispatcher_impl_unittest.cc

@@ -12,6 +12,7 @@
 #include "components/offline_pages/core/offline_page_feature.h"
 #include "components/offline_pages/core/prefetch/generate_page_bundle_request.h"
 #include "components/offline_pages/core/prefetch/get_operation_request.h"
+#include "components/offline_pages/core/prefetch/prefetch_configuration.h"
 #include "components/offline_pages/core/prefetch/prefetch_network_request_factory.h"
 #include "components/offline_pages/core/prefetch/prefetch_request_test_base.h"
 #include "components/offline_pages/core/prefetch/prefetch_service.h"
@@ -60,6 +61,10 @@ class PrefetchDispatcherTest : public PrefetchRequestTestBase {
     return dispatcher_->background_task_.get();
   }
 
+  TaskQueue& GetTaskQueueFrom(PrefetchDispatcherImpl* prefetch_dispatcher) {
+    return prefetch_dispatcher->task_queue_;
+  }
+
   TaskQueue* dispatcher_task_queue() { return &dispatcher_->task_queue_; }
   PrefetchDispatcher* prefetch_dispatcher() { return dispatcher_; }
   TestPrefetchNetworkRequestFactory* network_request_factory() {
@@ -130,18 +135,41 @@ TEST_F(PrefetchDispatcherTest, DispatcherDoesNothingIfFeatureNotEnabled) {
   disabled_feature_list.InitAndDisableFeature(kPrefetchingOfflinePagesFeature);
 
   // Don't add a task for new prefetch URLs.
-  PrefetchURL prefetch_url("id", GURL("https://www.chromium.org"),
-                           base::string16());
-  prefetch_dispatcher()->AddCandidatePrefetchURLs(
-      kTestNamespace, std::vector<PrefetchURL>(1, prefetch_url));
+  prefetch_dispatcher()->AddCandidatePrefetchURLs(kTestNamespace, test_urls_);
   EXPECT_FALSE(dispatcher_task_queue()->HasRunningTask());
 
   // Do nothing with a new background task.
   prefetch_dispatcher()->BeginBackgroundTask(
       base::MakeUnique<TestScopedBackgroundTask>());
   EXPECT_EQ(nullptr, GetBackgroundTask());
 
-  // Everything else is unimplemented.
+  // TODO(carlosk): add more checks here.
+}
+
+class DisablingPrefetchConfiguration : public PrefetchConfiguration {
+ public:
+  bool IsPrefetchingEnabledBySettings() override { return false; };
+};
+
+TEST_F(PrefetchDispatcherTest, DispatcherDoesNothingIfSettingsDoNotAllowIt) {
+  PrefetchDispatcherImpl* dispatcher = new PrefetchDispatcherImpl();
+  PrefetchServiceTestTaco taco;
+  taco.SetPrefetchDispatcher(base::WrapUnique(dispatcher));
+  taco.SetPrefetchNetworkRequestFactory(
+      base::MakeUnique<TestPrefetchNetworkRequestFactory>());
+  taco.SetPrefetchConfiguration(
+      base::MakeUnique<DisablingPrefetchConfiguration>());
+  taco.CreatePrefetchService();
+
+  // Don't add a task for new prefetch URLs.
+  dispatcher->AddCandidatePrefetchURLs(kTestNamespace, test_urls_);
+  EXPECT_FALSE(GetTaskQueueFrom(dispatcher).HasRunningTask());
+
+  // Do nothing with a new background task.
+  dispatcher->BeginBackgroundTask(base::MakeUnique<TestScopedBackgroundTask>());
+  EXPECT_EQ(nullptr, GetBackgroundTask());
+
+  // TODO(carlosk): add more checks here.
 }
 
 TEST_F(PrefetchDispatcherTest, DispatcherReleasesBackgroundTask) {

components/offline_pages/core/prefetch/prefetch_service.h

@@ -11,6 +11,7 @@ namespace offline_pages {
 class OfflineEventLogger;
 class OfflineMetricsCollector;
 class PrefetchBackgroundTaskHandler;
+class PrefetchConfiguration;
 class PrefetchDispatcher;
 class PrefetchDownloader;
 class PrefetchGCMHandler;
@@ -39,6 +40,7 @@ class PrefetchService : public KeyedService {
   virtual PrefetchStore* GetPrefetchStore() = 0;
   virtual PrefetchImporter* GetPrefetchImporter() = 0;
   virtual PrefetchBackgroundTaskHandler* GetPrefetchBackgroundTaskHandler() = 0;
+  virtual PrefetchConfiguration* GetPrefetchConfiguration() = 0;
 
   // May be |nullptr| in tests.  The PrefetchService does not depend on the
   // SuggestedArticlesObserver, it merely owns it for lifetime purposes.

components/offline_pages/core/prefetch/prefetch_service_impl.cc

@@ -12,6 +12,7 @@
 #include "components/offline_pages/core/client_namespace_constants.h"
 #include "components/offline_pages/core/prefetch/offline_metrics_collector.h"
 #include "components/offline_pages/core/prefetch/prefetch_background_task_handler.h"
+#include "components/offline_pages/core/prefetch/prefetch_configuration.h"
 #include "components/offline_pages/core/prefetch/prefetch_dispatcher.h"
 #include "components/offline_pages/core/prefetch/prefetch_downloader.h"
 #include "components/offline_pages/core/prefetch/prefetch_gcm_handler.h"
@@ -32,7 +33,8 @@ PrefetchServiceImpl::PrefetchServiceImpl(
     std::unique_ptr<PrefetchDownloader> prefetch_downloader,
     std::unique_ptr<PrefetchImporter> prefetch_importer,
     std::unique_ptr<PrefetchBackgroundTaskHandler>
-        prefetch_background_task_handler)
+        prefetch_background_task_handler,
+    std::unique_ptr<PrefetchConfiguration> prefetch_configuration)
     : offline_metrics_collector_(std::move(offline_metrics_collector)),
       prefetch_dispatcher_(std::move(dispatcher)),
       prefetch_gcm_handler_(std::move(gcm_handler)),
@@ -42,7 +44,8 @@ PrefetchServiceImpl::PrefetchServiceImpl(
       prefetch_downloader_(std::move(prefetch_downloader)),
       prefetch_importer_(std::move(prefetch_importer)),
       prefetch_background_task_handler_(
-          std::move(prefetch_background_task_handler)) {
+          std::move(prefetch_background_task_handler)),
+      prefetch_configuration_(std::move(prefetch_configuration)) {
   prefetch_dispatcher_->SetService(this);
   prefetch_downloader_->SetPrefetchService(this);
   prefetch_gcm_handler_->SetService(this);
@@ -93,6 +96,10 @@ PrefetchServiceImpl::GetPrefetchBackgroundTaskHandler() {
   return prefetch_background_task_handler_.get();
 }
 
+PrefetchConfiguration* PrefetchServiceImpl::GetPrefetchConfiguration() {
+  return prefetch_configuration_.get();
+}
+
 void PrefetchServiceImpl::Shutdown() {
   suggested_articles_observer_.reset();
   prefetch_downloader_.reset();