Be able to test the OfflineItem entry made for a Background Fetch

TBR=fgorski@ for namespace change affecting offline_page_download_bridge.cc

Change-Id: I2570e3c45827bb223cd3a729ee60e2e7c63b632c
Reviewed-on: https://chromium-review.googlesource.com/955844Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542612}

chrome/browser/android/download/items/offline_content_aggregator_factory_android.cc

@@ -22,8 +22,8 @@ JNI_OfflineContentAggregatorFactory_GetOfflineContentAggregatorForProfile(
   Profile* profile = ProfileAndroid::FromProfileAndroid(jprofile);
   DCHECK(profile);
   offline_items_collection::OfflineContentAggregator* aggregator =
-      offline_items_collection::OfflineContentAggregatorFactory::GetInstance()
-          ->GetForBrowserContext(profile);
+      OfflineContentAggregatorFactory::GetInstance()->GetForBrowserContext(
+          profile);
   return offline_items_collection::android::OfflineContentAggregatorBridge::
       GetBridgeForOfflineContentAggregator(aggregator);
 }

chrome/browser/background_fetch/background_fetch_browsertest.cc

@@ -8,7 +8,9 @@
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/run_loop.h"
+#include "base/strings/stringprintf.h"
 #include "chrome/browser/download/download_service_factory.h"
+#include "chrome/browser/offline_items_collection/offline_content_aggregator_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
@@ -16,10 +18,20 @@
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/download/public/background_service/download_service.h"
 #include "components/download/public/background_service/logger.h"
+#include "components/offline_items_collection/core/offline_content_aggregator.h"
+#include "components/offline_items_collection/core/offline_content_provider.h"
+#include "components/offline_items_collection/core/offline_item.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/browser_test_utils.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
+#include "url/origin.h"
+
+using offline_items_collection::ContentId;
+using offline_items_collection::OfflineContentProvider;
+using offline_items_collection::OfflineItem;
+using offline_items_collection::OfflineItemFilter;
+using offline_items_collection::OfflineItemProgressUnit;
 
 namespace {
 
@@ -32,6 +44,9 @@ const char kBackgroundFetchClient[] = "BackgroundFetch";
 // Stringified values of request services made to the download service.
 const char kResultAccepted[] = "ACCEPTED";
 
+// Title of a Background Fetch started by StartSingleFileDownload().
+const char kSingleFileDownloadTitle[] = "Single-file Background Fetch";
+
 // Implementation of a download system logger that provides the ability to wait
 // for certain events to happen, notably added and progressing downloads.
 class WaitableDownloadLoggerObserver : public download::Logger::Observer {
@@ -61,7 +76,7 @@ class WaitableDownloadLoggerObserver : public download::Logger::Observer {
     if (client != kBackgroundFetchClient)
       return;  // this event is not targeted to us
 
-    if (result == kResultAccepted)
+    if (result == kResultAccepted && download_accepted_callback_)
       std::move(download_accepted_callback_).Run(guid);
   }
 
@@ -71,9 +86,41 @@ class WaitableDownloadLoggerObserver : public download::Logger::Observer {
   DISALLOW_COPY_AND_ASSIGN(WaitableDownloadLoggerObserver);
 };
 
+// Observes the offline item collection's content provider and then invokes the
+// associated test callbacks when one has been provided.
+class OfflineContentProviderObserver : public OfflineContentProvider::Observer {
+ public:
+  using ItemsAddedCallback =
+      base::OnceCallback<void(const std::vector<OfflineItem>&)>;
+
+  OfflineContentProviderObserver() = default;
+  ~OfflineContentProviderObserver() final = default;
+
+  void set_items_added_callback(ItemsAddedCallback callback) {
+    items_added_callback_ = std::move(callback);
+  }
+
+  // OfflineContentProvider::Observer implementation:
+  void OnItemsAdded(
+      const OfflineContentProvider::OfflineItemList& items) override {
+    if (items_added_callback_)
+      std::move(items_added_callback_).Run(items);
+  }
+
+  void OnItemRemoved(const ContentId& id) override {}
+  void OnItemUpdated(const OfflineItem& item) override {}
+
+ private:
+  ItemsAddedCallback items_added_callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(OfflineContentProviderObserver);
+};
+
 class BackgroundFetchBrowserTest : public InProcessBrowserTest {
  public:
-  BackgroundFetchBrowserTest() = default;
+  BackgroundFetchBrowserTest()
+      : offline_content_provider_observer_(
+            std::make_unique<OfflineContentProviderObserver>()) {}
   ~BackgroundFetchBrowserTest() override = default;
 
   // InProcessBrowserTest overrides:
@@ -89,11 +136,17 @@ class BackgroundFetchBrowserTest : public InProcessBrowserTest {
     https_server_->ServeFilesFromSourceDirectory("chrome/test/data");
     ASSERT_TRUE(https_server_->Start());
 
-    observer_ = std::make_unique<WaitableDownloadLoggerObserver>();
+    Profile* profile = browser()->profile();
+
+    download_observer_ = std::make_unique<WaitableDownloadLoggerObserver>();
 
-    download_service_ =
-        DownloadServiceFactory::GetForBrowserContext(browser()->profile());
-    download_service_->GetLogger()->AddObserver(observer_.get());
+    download_service_ = DownloadServiceFactory::GetForBrowserContext(profile);
+    download_service_->GetLogger()->AddObserver(download_observer_.get());
+
+    // Register our observer for the offline items collection.
+    OfflineContentAggregatorFactory::GetInstance()
+        ->GetForBrowserContext(profile)
+        ->AddObserver(offline_content_provider_observer_.get());
 
     // Load the helper page that helps drive these tests.
     ui_test_utils::NavigateToURL(browser(), https_server_->GetURL(kHelperPage));
@@ -108,10 +161,46 @@ class BackgroundFetchBrowserTest : public InProcessBrowserTest {
   }
 
   void TearDownOnMainThread() override {
-    download_service_->GetLogger()->RemoveObserver(observer_.get());
+    OfflineContentAggregatorFactory::GetInstance()
+        ->GetForBrowserContext(browser()->profile())
+        ->RemoveObserver(offline_content_provider_observer_.get());
+
+    download_service_->GetLogger()->RemoveObserver(download_observer_.get());
     download_service_ = nullptr;
   }
 
+  // ---------------------------------------------------------------------------
+  // Test execution functions.
+
+  // Runs the |script| and waits for one or more items to have been added to the
+  // offline items collection. Wrap in ASSERT_NO_FATAL_FAILURE().
+  void RunScriptAndWaitForOfflineItems(const std::string& script,
+                                       std::vector<OfflineItem>* items) {
+    DCHECK(items);
+
+    base::RunLoop run_loop;
+    offline_content_provider_observer_->set_items_added_callback(
+        base::BindOnce(&BackgroundFetchBrowserTest::DidAddItems,
+                       base::Unretained(this), run_loop.QuitClosure(), items));
+
+    std::string result;
+    ASSERT_NO_FATAL_FAILURE(RunScript(script, &result));
+    ASSERT_EQ("ok", result);
+
+    run_loop.Run();
+  }
+
+  // ---------------------------------------------------------------------------
+  // Helper functions.
+
+  // Helper function for getting the expected title given the |developer_title|.
+  std::string GetExpectedTitle(const char* developer_title) {
+    url::Origin origin = url::Origin::Create(https_server_->base_url());
+
+    return base::StringPrintf("%s (%s)", developer_title,
+                              origin.Serialize().c_str());
+  }
+
   // Runs the |script| in the current tab and writes the output to |*result|.
   bool RunScript(const std::string& script, std::string* result) {
     return content::ExecuteScriptAndExtractString(
@@ -139,22 +228,34 @@ class BackgroundFetchBrowserTest : public InProcessBrowserTest {
 
  protected:
   download::DownloadService* download_service_{nullptr};
-  std::unique_ptr<WaitableDownloadLoggerObserver> observer_;
+
+  std::unique_ptr<WaitableDownloadLoggerObserver> download_observer_;
+  std::unique_ptr<OfflineContentProviderObserver>
+      offline_content_provider_observer_;
 
  private:
+  // Callback for RunScriptAndWaitForOfflineItems(), called when the |items|
+  // have been added to the offline items collection.
+  void DidAddItems(base::OnceClosure quit_closure,
+                   std::vector<OfflineItem>* out_items,
+                   const std::vector<OfflineItem>& items) {
+    *out_items = items;
+    std::move(quit_closure).Run();
+  }
+
   std::unique_ptr<net::EmbeddedTestServer> https_server_;
 
   DISALLOW_COPY_AND_ASSIGN(BackgroundFetchBrowserTest);
 };
 
-IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest, DownloadSingleFile) {
+IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest, DownloadService_Acceptance) {
   // Starts a Background Fetch for a single to-be-downloaded file and waits for
   // that request to be scheduled with the Download Service.
 
   std::string guid;
   {
     base::RunLoop run_loop;
-    observer_->set_download_accepted_callback(
+    download_observer_->set_download_accepted_callback(
         base::BindOnce(&BackgroundFetchBrowserTest::DidAcceptDownloadCallback,
                        base::Unretained(this), run_loop.QuitClosure(), &guid));
 
@@ -165,4 +266,34 @@ IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest, DownloadSingleFile) {
   EXPECT_FALSE(guid.empty());
 }
 
+IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest,
+                       OfflineItemCollection_SingleFileMetadata) {
+  // Starts a Background Fetch for a single to-be-downloaded file and waits for
+  // the fetch to be registered with the offline items collection. We then
+  // verify that all the appropriate values have been set.
+
+  std::vector<OfflineItem> items;
+  ASSERT_NO_FATAL_FAILURE(
+      RunScriptAndWaitForOfflineItems("StartSingleFileDownload()", &items));
+  ASSERT_EQ(items.size(), 1u);
+
+  const OfflineItem& offline_item = items[0];
+
+  // Verify that the appropriate data is being set.
+  EXPECT_EQ(offline_item.title, GetExpectedTitle(kSingleFileDownloadTitle));
+  EXPECT_EQ(offline_item.filter, OfflineItemFilter::FILTER_OTHER);
+  EXPECT_TRUE(offline_item.is_transient);
+  EXPECT_FALSE(offline_item.is_suggested);
+  EXPECT_FALSE(offline_item.is_off_the_record);
+
+  EXPECT_EQ(offline_item.progress.value, 0);
+  EXPECT_EQ(offline_item.progress.max, 1);
+  EXPECT_EQ(offline_item.progress.unit, OfflineItemProgressUnit::PERCENTAGE);
+
+  // Change-detector tests for values we might want to provide or change.
+  EXPECT_TRUE(offline_item.description.empty());
+  EXPECT_TRUE(offline_item.page_url.is_empty());
+  EXPECT_FALSE(offline_item.is_resumable);
+}
+
 }  // namespace

chrome/browser/background_fetch/background_fetch_delegate_impl.cc

@@ -24,8 +24,7 @@ BackgroundFetchDelegateImpl::BackgroundFetchDelegateImpl(Profile* profile)
     : download_service_(
           DownloadServiceFactory::GetInstance()->GetForBrowserContext(profile)),
       offline_content_aggregator_(
-          offline_items_collection::OfflineContentAggregatorFactory::
-              GetForBrowserContext(profile)),
+          OfflineContentAggregatorFactory::GetForBrowserContext(profile)),
       weak_ptr_factory_(this) {
   offline_content_aggregator_->RegisterProvider("background_fetch", this);
 }

chrome/browser/offline_items_collection/offline_content_aggregator_factory.cc

@@ -10,8 +10,6 @@
 #include "components/offline_items_collection/core/offline_content_aggregator.h"
 #include "content/public/browser/browser_context.h"
 
-namespace offline_items_collection {
-
 // static
 OfflineContentAggregatorFactory*
 OfflineContentAggregatorFactory::GetInstance() {
@@ -19,7 +17,8 @@ OfflineContentAggregatorFactory::GetInstance() {
 }
 
 // static
-OfflineContentAggregator* OfflineContentAggregatorFactory::GetForBrowserContext(
+offline_items_collection::OfflineContentAggregator*
+OfflineContentAggregatorFactory::GetForBrowserContext(
     content::BrowserContext* context) {
   DCHECK(!context->IsOffTheRecord());
   return static_cast<offline_items_collection::OfflineContentAggregator*>(
@@ -43,5 +42,3 @@ OfflineContentAggregatorFactory::GetBrowserContextToUse(
     content::BrowserContext* context) const {
   return chrome::GetBrowserContextRedirectedInIncognito(context);
 }
-
-}  // namespace offline_items_collection

chrome/browser/offline_items_collection/offline_content_aggregator_factory.h

@@ -19,6 +19,7 @@ class BrowserContext;
 
 namespace offline_items_collection {
 class OfflineContentAggregator;
+}  // namespace offline_items_collection
 
 // This class is the main access point for an OfflineContentAggregator.  It is
 // responsible for building the OfflineContentAggregator and associating it with
@@ -49,6 +50,4 @@ class OfflineContentAggregatorFactory
   DISALLOW_COPY_AND_ASSIGN(OfflineContentAggregatorFactory);
 };
 
-}  // namespace offline_items_collection
-
 #endif  // CHROME_BROWSER_OFFLINE_ITEMS_COLLECTION_OFFLINE_CONTENT_AGGREGATOR_FACTORY_H_

chrome/browser/offline_pages/android/downloads/offline_page_download_bridge.cc

@@ -343,8 +343,7 @@ static jlong JNI_OfflinePageDownloadBridge_Init(
         RequestCoordinatorFactory::GetForBrowserContext(browser_context);
     DCHECK(request_coordinator);
     offline_items_collection::OfflineContentAggregator* aggregator =
-        offline_items_collection::OfflineContentAggregatorFactory::
-            GetForBrowserContext(browser_context);
+        OfflineContentAggregatorFactory::GetForBrowserContext(browser_context);
     DCHECK(aggregator);
     adapter = new DownloadUIAdapter(
         aggregator, offline_page_model, request_coordinator,

chrome/test/data/background_fetch/background_fetch.js

@@ -14,8 +14,13 @@ function RegisterServiceWorker() {
 // Starts a Background Fetch request for a single to-be-downloaded file.
 function StartSingleFileDownload() {
   navigator.serviceWorker.ready.then(swRegistration => {
+    const options = {
+      // TODO(nator): Provide an icon here.
+      title: 'Single-file Background Fetch'
+    };
+
     return swRegistration.backgroundFetch.fetch(
-        kBackgroundFetchId, [ '/notifications/icon.png' ]);
+        kBackgroundFetchId, [ '/notifications/icon.png' ], options);
   }).then(bgFetchRegistration => {
     sendResultToTest('ok');
   }).catch(sendErrorToTest);