Allow embedder to use custom ResourceRequestAllowedNotifier

By allowing custom ResourceRequestAllowedNotifier, embedder
has the flexibility to set custom criteria when the fetch is
allowed or not allowed.

BUG=634458

Review-Url: https://codereview.chromium.org/2217683002
Cr-Commit-Position: refs/heads/master@{#454739}

AUTHORS

@@ -82,6 +82,7 @@ Arun Mankuzhi <arun.m@samsung.com>
 Arunoday Sarkar <a.sarkar.arun@gmail.com>
 Arunprasad Rajkumar <ararunprasad@gmail.com>
 Arunprasad Rajkumar <arurajku@cisco.com>
+Ashish Kumar Gupta <guptaag@amazon.com>
 Ashlin Joseph <ashlin.j@samsung.com>
 Attila Dusnoki <dati91@gmail.com>
 Avinaash Doreswamy <avi.nitk@samsung.com>

components/web_resource/BUILD.gn

@@ -43,6 +43,7 @@ source_set("unit_tests") {
   sources = [
     "eula_accepted_notifier_unittest.cc",
     "resource_request_allowed_notifier_unittest.cc",
+    "web_resource_service_unittest.cc",
   ]
 
   deps = [

components/web_resource/web_resource_service.cc

@@ -44,7 +44,9 @@ WebResourceService::WebResourceService(
     const char* disable_network_switch,
     const ParseJSONCallback& parse_json_callback)
     : prefs_(prefs),
-      resource_request_allowed_notifier_(prefs, disable_network_switch),
+      resource_request_allowed_notifier_(
+          new ResourceRequestAllowedNotifier(prefs, disable_network_switch)),
+      fetch_scheduled_(false),
       in_fetch_(false),
       web_resource_server_(web_resource_server),
       application_locale_(application_locale),
@@ -54,13 +56,13 @@ WebResourceService::WebResourceService(
       request_context_(request_context),
       parse_json_callback_(parse_json_callback),
       weak_ptr_factory_(this) {
-  resource_request_allowed_notifier_.Init(this);
+  resource_request_allowed_notifier_->Init(this);
   DCHECK(prefs);
 }
 
 void WebResourceService::StartAfterDelay() {
   // If resource requests are not allowed, we'll get a callback when they are.
-  if (resource_request_allowed_notifier_.ResourceRequestsAllowed())
+  if (resource_request_allowed_notifier_->ResourceRequestsAllowed())
     OnResourceRequestsAllowed();
 }
 
@@ -100,15 +102,35 @@ void WebResourceService::OnURLFetchComplete(const net::URLFetcher* source) {
 // Delay initial load of resource data into cache so as not to interfere
 // with startup time.
 void WebResourceService::ScheduleFetch(int64_t delay_ms) {
+  if (fetch_scheduled_)
+    return;
+  fetch_scheduled_ = true;
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE, base::Bind(&WebResourceService::StartFetch,
                             weak_ptr_factory_.GetWeakPtr()),
       base::TimeDelta::FromMilliseconds(delay_ms));
 }
 
+void WebResourceService::SetResourceRequestAllowedNotifier(
+    std::unique_ptr<ResourceRequestAllowedNotifier> notifier) {
+  resource_request_allowed_notifier_ = std::move(notifier);
+  resource_request_allowed_notifier_->Init(this);
+}
+
+bool WebResourceService::GetFetchScheduled() const {
+  return fetch_scheduled_;
+}
+
 // Initializes the fetching of data from the resource server.  Data
 // load calls OnURLFetchComplete.
 void WebResourceService::StartFetch() {
+  // Set to false so that next fetch can be scheduled after this fetch or
+  // if we recieve notification that resource is allowed.
+  fetch_scheduled_ = false;
+  // Check whether fetching is allowed.
+  if (!resource_request_allowed_notifier_->ResourceRequestsAllowed())
+    return;
+
   // First, put our next cache load on the MessageLoop.
   ScheduleFetch(cache_update_delay_ms_);
 

components/web_resource/web_resource_service.h

@@ -64,10 +64,17 @@ class WebResourceService
   // Then begin updating resources.
   void StartAfterDelay();
 
+  // Sets the ResourceRequestAllowedNotifier to make it configurable.
+  void SetResourceRequestAllowedNotifier(
+      std::unique_ptr<ResourceRequestAllowedNotifier> notifier);
+
  protected:
   PrefService* prefs_;
+  bool GetFetchScheduled() const;
 
  private:
+  friend class WebResourceServiceTest;
+
   // For the subclasses to process the result of a fetch.
   virtual void Unpack(const base::DictionaryValue& parsed_json) = 0;
 
@@ -92,7 +99,15 @@ class WebResourceService
 
   // Helper class used to tell this service if it's allowed to make network
   // resource requests.
-  ResourceRequestAllowedNotifier resource_request_allowed_notifier_;
+  std::unique_ptr<ResourceRequestAllowedNotifier>
+      resource_request_allowed_notifier_;
+
+  // True if we have scheduled a fetch after start_fetch_delay_ms_
+  // or another one in |cache_update_delay_ms_| time. Set to false
+  // before fetching starts so that next fetch is scheduled. This
+  // is to make sure not more than one fetch is scheduled for given
+  // point in time.
+  bool fetch_scheduled_;
 
   // The tool that fetches the url data from the server.
   std::unique_ptr<net::URLFetcher> url_fetcher_;

components/web_resource/web_resource_service_unittest.cc

@@ -0,0 +1,154 @@
+// 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 <memory>
+#include <utility>
+
+#include "base/bind.h"
+#include "base/message_loop/message_loop.h"
+#include "base/values.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/testing_pref_service.h"
+#include "components/web_resource/resource_request_allowed_notifier.h"
+#include "components/web_resource/web_resource_service.h"
+#include "net/url_request/test_url_fetcher_factory.h"
+#include "net/url_request/url_request_context_getter.h"
+#include "net/url_request/url_request_status.h"
+#include "net/url_request/url_request_test_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+const std::string kTestUrl = "http://www.test.com";
+const std::string kCacheUpdatePath = "cache_update_path";
+std::string error_message_;
+}  // namespace
+
+namespace web_resource {
+
+class TestResourceRequestAllowedNotifier
+    : public ResourceRequestAllowedNotifier {
+ public:
+  TestResourceRequestAllowedNotifier(PrefService* prefs,
+                                     const char* disable_network_switch)
+      : ResourceRequestAllowedNotifier(prefs, disable_network_switch) {}
+
+  ResourceRequestAllowedNotifier::State GetResourceRequestsAllowedState()
+      override {
+    return state_;
+  }
+
+  void SetState(ResourceRequestAllowedNotifier::State state) { state_ = state; }
+
+  void NotifyState(ResourceRequestAllowedNotifier::State state) {
+    SetState(state);
+    SetObserverRequestedForTesting(true);
+    MaybeNotifyObserver();
+  }
+
+ private:
+  ResourceRequestAllowedNotifier::State state_;
+};
+
+class TestWebResourceService : public WebResourceService {
+ public:
+  TestWebResourceService(PrefService* prefs,
+                         const GURL& web_resource_server,
+                         const std::string& application_locale,
+                         const char* last_update_time_pref_name,
+                         int start_fetch_delay_ms,
+                         int cache_update_delay_ms,
+                         net::URLRequestContextGetter* request_context,
+                         const char* disable_network_switch,
+                         const ParseJSONCallback& parse_json_callback)
+      : WebResourceService(prefs,
+                           web_resource_server,
+                           application_locale,
+                           last_update_time_pref_name,
+                           start_fetch_delay_ms,
+                           cache_update_delay_ms,
+                           request_context,
+                           disable_network_switch,
+                           parse_json_callback){};
+
+  void Unpack(const base::DictionaryValue& parsed_json) override{};
+};
+
+class WebResourceServiceTest : public testing::Test {
+ public:
+  WebResourceServiceTest() {}
+
+  void SetUp() override {
+    request_context_getter_ = new net::TestURLRequestContextGetter(
+        base::ThreadTaskRunnerHandle::Get());
+    local_state_.reset(new TestingPrefServiceSimple());
+    local_state_->registry()->RegisterStringPref(kCacheUpdatePath, "0");
+    test_web_resource_service_.reset(new TestWebResourceService(
+        local_state_.get(), GURL(kTestUrl), "", kCacheUpdatePath.c_str(), 100,
+        5000, request_context_getter_.get(), nullptr,
+        base::Bind(web_resource::WebResourceServiceTest::Parse)));
+    error_message_ = "";
+    TestResourceRequestAllowedNotifier* notifier =
+        new TestResourceRequestAllowedNotifier(local_state_.get(), nullptr);
+    notifier->SetState(ResourceRequestAllowedNotifier::ALLOWED);
+    test_web_resource_service_->SetResourceRequestAllowedNotifier(
+        std::unique_ptr<ResourceRequestAllowedNotifier>(notifier));
+  }
+
+  TestResourceRequestAllowedNotifier* resource_notifier() {
+    return static_cast<TestResourceRequestAllowedNotifier*>(
+        test_web_resource_service_->resource_request_allowed_notifier_.get());
+  }
+
+  bool GetFetchScheduled() {
+    return test_web_resource_service_->GetFetchScheduled();
+  }
+
+  void CallScheduleFetch(int64_t delay_ms) {
+    return test_web_resource_service_->ScheduleFetch(delay_ms);
+  }
+
+  static void Parse(const std::string& unsafe_json,
+                    const WebResourceService::SuccessCallback& success_callback,
+                    const WebResourceService::ErrorCallback& error_callback) {
+    std::unique_ptr<base::Value> value;
+    if (!error_message_.empty())
+      error_callback.Run(error_message_);
+    else
+      success_callback.Run(std::move(value));
+  }
+
+  WebResourceService* web_resource_service() {
+    return test_web_resource_service_.get();
+  }
+
+  void CallStartFetch() { test_web_resource_service_->StartFetch(); }
+
+ private:
+  base::MessageLoop message_loop_;  // needed for TestURLFetcherFactory
+  scoped_refptr<net::URLRequestContextGetter> request_context_getter_;
+  std::unique_ptr<TestingPrefServiceSimple> local_state_;
+  std::unique_ptr<TestWebResourceService> test_web_resource_service_;
+};
+
+TEST_F(WebResourceServiceTest, FetchScheduledAfterStartDelayTest) {
+  web_resource_service()->StartAfterDelay();
+  EXPECT_TRUE(GetFetchScheduled());
+}
+
+TEST_F(WebResourceServiceTest, FetchScheduledOnScheduleFetchTest) {
+  web_resource_service()->StartAfterDelay();
+  resource_notifier()->NotifyState(ResourceRequestAllowedNotifier::ALLOWED);
+  EXPECT_TRUE(GetFetchScheduled());
+}
+
+TEST_F(WebResourceServiceTest, FetchScheduledOnStartFetchTest) {
+  resource_notifier()->NotifyState(
+      ResourceRequestAllowedNotifier::DISALLOWED_NETWORK_DOWN);
+  CallStartFetch();
+  EXPECT_FALSE(GetFetchScheduled());
+  resource_notifier()->NotifyState(ResourceRequestAllowedNotifier::ALLOWED);
+  EXPECT_TRUE(GetFetchScheduled());
+}
+
+}  // namespace web_resource