Reland "Eliminate unnecessary proto::Configuration copies"

This is a reland of 1b420fdb

The flakiness in the original CL with ResourceLoadingHintsBrowserTest
and PreviewsNoScriptBrowserTest has been fixed.
RetryForHistogramUntilCountReached() now flushes the task scheduler
and keeps trying until the test times out.

Original change's description:
> Eliminate unnecessary proto::Configuration copies
>
> The previous logic in OptimizationGuideService created an
> optimization_guide::proto::Configuration object on a background thread
> and then used task posting to send it on to
> PreviewsHints::CreateFromConfig(), where it was used to create a
> PreviewsHints object on another background thread. In all, there were
> two tasks posted that included the config object as a parameter, one on
> the background thread and one on the UI thread. Including it as a
> parameter in a task requires a full copy of the protobuffer. This means
> that a full copy of the config protobuf, which can be as large as 1.8MB
> (the size of the current Brazil one on Canary), was occurring on the UI
> thread.
>
> In local performance measurements, making a single copy of a 600KB
> version of the protobuf took several milliseconds (it amounted to
> roughly 60% of the time taken by the initial component string parsing
> and 60% of the time taken by PreviewHints::CreateFromConfig()). Given
> that we were incurring the cost of two copies, one of which was on an
> extremely high priority thread, it makes sense to change the logic to
> eliminate the need for the copies.
>
> The logic has been altered so that OptimizationGuideService no longer
> immediately processes the component, but instead notifies the
> observers that it is available and allows them to trigger the
> processing. This eliminates both copies of the configuration protobuf,
> as it is now created where it is used.
>
> Additionally, OptimizationGuideServiceObservers are now immediately
> notified of the hints component when they register if one is already
> available. This will enable the PreviewsOptimizationGuide to wait until
> the HintCacheLevelDBStore is fully initialized before registering for
> the component, and in the future will potentially allow it to avoid
> processing the configuration altogether when the store already has the
> latest version cached.
>
> New unittests have been added and older ones have been updated to
> accommodate the new logic.
>
> The related browser tests have also been modified to be more robust,
> where they now wait for a signal from a local histogram indicating
> that hint processing is complete.

Bug: 908985, 910251
Change-Id: I90407db4c19dac29e10f756a6de87294a9ab683b
Reviewed-on: https://chromium-review.googlesource.com/c/1355256
Commit-Queue: Jered Gray <jegray@chromium.org>
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612408}

chrome/browser/component_updater/optimization_hints_component_installer.cc

@@ -90,10 +90,10 @@ void OptimizationHintsComponentInstallerPolicy::ComponentReady(
   optimization_guide::OptimizationGuideService* optimization_guide_service =
       g_browser_process->optimization_guide_service();
   if (optimization_guide_service) {
-    optimization_guide::ComponentInfo component_info(
+    optimization_guide::HintsComponentInfo info(
         version,
         install_dir.Append(optimization_guide::kUnindexedHintsFileName));
-    optimization_guide_service->ProcessHints(component_info);
+    optimization_guide_service->MaybeUpdateHintsComponent(info);
   }
 }
 

chrome/browser/component_updater/optimization_hints_component_installer_unittest.cc

@@ -37,18 +37,18 @@ class TestOptimizationGuideService
       : optimization_guide::OptimizationGuideService(io_thread_task_runner) {}
   ~TestOptimizationGuideService() override {}
 
-  void ProcessHints(
-      const optimization_guide::ComponentInfo& component_info) override {
-    component_info_ =
-        std::make_unique<optimization_guide::ComponentInfo>(component_info);
+  void MaybeUpdateHintsComponent(
+      const optimization_guide::HintsComponentInfo& info) override {
+    hints_component_info_ =
+        std::make_unique<optimization_guide::HintsComponentInfo>(info);
   }
 
-  optimization_guide::ComponentInfo* component_info() const {
-    return component_info_.get();
+  optimization_guide::HintsComponentInfo* hints_component_info() const {
+    return hints_component_info_.get();
   }
 
  private:
-  std::unique_ptr<optimization_guide::ComponentInfo> component_info_;
+  std::unique_ptr<optimization_guide::HintsComponentInfo> hints_component_info_;
 
   DISALLOW_COPY_AND_ASSIGN(TestOptimizationGuideService);
 };
@@ -204,7 +204,7 @@ TEST_F(OptimizationHintsComponentInstallerTest, NoRulesetFormatIgnored) {
   ASSERT_NO_FATAL_FAILURE(CreateTestOptimizationHints("some hints"));
 
   ASSERT_NO_FATAL_FAILURE(LoadOptimizationHints(base::Version("")));
-  EXPECT_EQ(nullptr, service()->component_info());
+  EXPECT_EQ(nullptr, service()->hints_component_info());
 }
 
 TEST_F(OptimizationHintsComponentInstallerTest, FutureRulesetFormatIgnored) {
@@ -217,7 +217,7 @@ TEST_F(OptimizationHintsComponentInstallerTest, FutureRulesetFormatIgnored) {
 
   ASSERT_NO_FATAL_FAILURE(
       LoadOptimizationHints(base::Version(future_ruleset_components)));
-  EXPECT_EQ(nullptr, service()->component_info());
+  EXPECT_EQ(nullptr, service()->hints_component_info());
 }
 
 TEST_F(OptimizationHintsComponentInstallerTest, LoadFileWithData) {
@@ -227,12 +227,11 @@ TEST_F(OptimizationHintsComponentInstallerTest, LoadFileWithData) {
   ASSERT_NO_FATAL_FAILURE(CreateTestOptimizationHints(expected_hints));
   ASSERT_NO_FATAL_FAILURE(LoadOptimizationHints(ruleset_format_version()));
 
-  auto* component_info = service()->component_info();
+  auto* component_info = service()->hints_component_info();
   EXPECT_NE(nullptr, component_info);
-  EXPECT_EQ(base::Version(kTestHintsVersion), component_info->hints_version);
+  EXPECT_EQ(base::Version(kTestHintsVersion), component_info->version);
   std::string actual_hints;
-  ASSERT_TRUE(
-      base::ReadFileToString(component_info->hints_path, &actual_hints));
+  ASSERT_TRUE(base::ReadFileToString(component_info->path, &actual_hints));
   EXPECT_EQ(expected_hints, actual_hints);
 }
 

chrome/browser/previews/previews_browsertest.cc

@@ -7,19 +7,22 @@
 #include "base/metrics/field_trial_params.h"
 #include "base/run_loop.h"
 #include "base/task/post_task.h"
+#include "base/task/task_scheduler/task_scheduler.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/metrics/subprocess_metrics_provider.h"
 #include "chrome/browser/previews/previews_ui_tab_helper.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_features.h"
+#include "components/optimization_guide/hints_component_info.h"
 #include "components/optimization_guide/optimization_guide_service.h"
-#include "components/optimization_guide/optimization_guide_service_observer.h"
 #include "components/optimization_guide/proto/hints.pb.h"
-#include "components/optimization_guide/test_component_creator.h"
+#include "components/optimization_guide/test_hints_component_creator.h"
+#include "components/previews/core/previews_constants.h"
 #include "components/previews/core/previews_features.h"
 #include "components/previews/core/previews_switches.h"
 #include "content/public/browser/browser_task_traits.h"
@@ -30,32 +33,28 @@
 
 namespace {
 
-// A test observer which can be configured to wait until the server hints are
-// processed.
-class TestOptimizationGuideServiceObserver
-    : public optimization_guide::OptimizationGuideServiceObserver {
- public:
-  TestOptimizationGuideServiceObserver()
-      : run_loop_(std::make_unique<base::RunLoop>()) {}
-
-  ~TestOptimizationGuideServiceObserver() override {}
+// Retries fetching |histogram_name| until it contains at least |count| samples.
+void RetryForHistogramUntilCountReached(base::HistogramTester* histogram_tester,
+                                        const std::string& histogram_name,
+                                        size_t count) {
+  while (true) {
+    base::TaskScheduler::GetInstance()->FlushForTesting();
+    base::RunLoop().RunUntilIdle();
 
-  void WaitForNotification() {
-    run_loop_->Run();
-    run_loop_.reset(new base::RunLoop());
-  }
+    content::FetchHistogramsFromChildProcesses();
+    SubprocessMetricsProvider::MergeHistogramDeltasForTesting();
 
- private:
-  void OnHintsProcessed(
-      const optimization_guide::proto::Configuration& config,
-      const optimization_guide::ComponentInfo& component_info) override {
-    run_loop_->Quit();
+    const std::vector<base::Bucket> buckets =
+        histogram_tester->GetAllSamples(histogram_name);
+    size_t total_count = 0;
+    for (const auto& bucket : buckets) {
+      total_count += bucket.count;
+    }
+    if (total_count >= count) {
+      break;
+    }
   }
-
-  std::unique_ptr<base::RunLoop> run_loop_;
-
-  DISALLOW_COPY_AND_ASSIGN(TestOptimizationGuideServiceObserver);
-};
+}
 
 }  // namespace
 
@@ -213,25 +212,26 @@ class PreviewsNoScriptBrowserTest : public PreviewsBrowserTest {
 
   void SetUpNoScriptWhitelist(
       std::vector<std::string> whitelisted_noscript_sites) {
-    TestOptimizationGuideServiceObserver observer;
-    g_browser_process->optimization_guide_service()->AddObserver(&observer);
-    base::RunLoop().RunUntilIdle();
-
-    const optimization_guide::ComponentInfo& component_info =
-        test_component_creator_.CreateComponentInfoWithPageHints(
+    const optimization_guide::HintsComponentInfo& component_info =
+        test_hints_component_creator_.CreateHintsComponentInfoWithPageHints(
             optimization_guide::proto::NOSCRIPT, whitelisted_noscript_sites,
             {});
-    g_browser_process->optimization_guide_service()->ProcessHints(
+
+    base::HistogramTester histogram_tester;
+
+    g_browser_process->optimization_guide_service()->MaybeUpdateHintsComponent(
         component_info);
 
-    // Wait for hints to be processed by PreviewsOptimizationGuide.
-    observer.WaitForNotification();
-    base::RunLoop().RunUntilIdle();
+    RetryForHistogramUntilCountReached(
+        &histogram_tester,
+        previews::kPreviewsOptimizationGuideUpdateHintsResultHistogramString,
+        1);
   }
 
  private:
   base::test::ScopedFeatureList scoped_feature_list_;
-  optimization_guide::testing::TestComponentCreator test_component_creator_;
+  optimization_guide::testing::TestHintsComponentCreator
+      test_hints_component_creator_;
 };
 
 // Previews InfoBar (which these tests triggers) does not work on Mac.

chrome/browser/previews/resource_loading_hints/resource_loading_hints_browsertest.cc

@@ -9,6 +9,7 @@
 #include "base/command_line.h"
 #include "base/run_loop.h"
 #include "base/task/post_task.h"
+#include "base/task/task_scheduler/task_scheduler.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
@@ -20,12 +21,13 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_features.h"
+#include "components/optimization_guide/hints_component_info.h"
 #include "components/optimization_guide/optimization_guide_service.h"
-#include "components/optimization_guide/optimization_guide_service_observer.h"
 #include "components/optimization_guide/proto/hints.pb.h"
-#include "components/optimization_guide/test_component_creator.h"
+#include "components/optimization_guide/test_hints_component_creator.h"
 #include "components/previews/content/previews_ui_service.h"
 #include "components/previews/core/previews_black_list.h"
+#include "components/previews/core/previews_constants.h"
 #include "components/previews/core/previews_features.h"
 #include "components/previews/core/previews_switches.h"
 #include "content/public/browser/browser_task_traits.h"
@@ -36,49 +38,26 @@
 
 namespace {
 
-// A test observer which can be configured to wait until the server hints are
-// processed.
-class TestOptimizationGuideServiceObserver
-    : public optimization_guide::OptimizationGuideServiceObserver {
- public:
-  TestOptimizationGuideServiceObserver()
-      : run_loop_(std::make_unique<base::RunLoop>()) {}
-
-  ~TestOptimizationGuideServiceObserver() override {}
-
-  void WaitForNotification() {
-    run_loop_->Run();
-    run_loop_.reset(new base::RunLoop());
-  }
-
- private:
-  void OnHintsProcessed(
-      const optimization_guide::proto::Configuration& config,
-      const optimization_guide::ComponentInfo& component_info) override {
-    run_loop_->Quit();
-  }
-
-  std::unique_ptr<base::RunLoop> run_loop_;
-
-  DISALLOW_COPY_AND_ASSIGN(TestOptimizationGuideServiceObserver);
-};
-
 // Retries fetching |histogram_name| until it contains at least |count| samples.
 void RetryForHistogramUntilCountReached(base::HistogramTester* histogram_tester,
                                         const std::string& histogram_name,
                                         size_t count) {
-  base::RunLoop().RunUntilIdle();
-  for (size_t attempt = 0; attempt < 3; ++attempt) {
+  while (true) {
+    base::TaskScheduler::GetInstance()->FlushForTesting();
+    base::RunLoop().RunUntilIdle();
+
+    content::FetchHistogramsFromChildProcesses();
+    SubprocessMetricsProvider::MergeHistogramDeltasForTesting();
+
     const std::vector<base::Bucket> buckets =
         histogram_tester->GetAllSamples(histogram_name);
     size_t total_count = 0;
-    for (const auto& bucket : buckets)
+    for (const auto& bucket : buckets) {
       total_count += bucket.count;
-    if (total_count >= count)
-      return;
-    content::FetchHistogramsFromChildProcesses();
-    SubprocessMetricsProvider::MergeHistogramDeltasForTesting();
-    base::RunLoop().RunUntilIdle();
+    }
+    if (total_count >= count) {
+      break;
+    }
   }
 }
 
@@ -140,6 +119,19 @@ class ResourceLoadingNoFeaturesBrowserTest : public InProcessBrowserTest {
     cmd->AppendSwitch(previews::switches::kIgnorePreviewsBlacklist);
   }
 
+  void ProcessHintsComponent(
+      const optimization_guide::HintsComponentInfo& component_info) {
+    base::HistogramTester histogram_tester;
+
+    g_browser_process->optimization_guide_service()->MaybeUpdateHintsComponent(
+        component_info);
+
+    RetryForHistogramUntilCountReached(
+        &histogram_tester,
+        previews::kPreviewsOptimizationGuideUpdateHintsResultHistogramString,
+        1);
+  }
+
   void SetDefaultOnlyResourceLoadingHints(
       const std::vector<std::string>& hints_sites) {
     std::vector<std::string> resource_patterns;
@@ -147,16 +139,10 @@ class ResourceLoadingNoFeaturesBrowserTest : public InProcessBrowserTest {
     resource_patterns.push_back("png");
     resource_patterns.push_back("woff2");
 
-    const optimization_guide::ComponentInfo& component_info =
-        test_component_creator_.CreateComponentInfoWithPageHints(
+    ProcessHintsComponent(
+        test_hints_component_creator_.CreateHintsComponentInfoWithPageHints(
             optimization_guide::proto::RESOURCE_LOADING, hints_sites,
-            resource_patterns);
-
-    g_browser_process->optimization_guide_service()->ProcessHints(
-        component_info);
-
-    // Wait for hints to be processed by PreviewsOptimizationGuide.
-    base::RunLoop().RunUntilIdle();
+            resource_patterns));
   }
 
   // Sets the resource loading hints in optimization guide service. The hints
@@ -168,16 +154,11 @@ class ResourceLoadingNoFeaturesBrowserTest : public InProcessBrowserTest {
     resource_patterns.push_back("png");
     resource_patterns.push_back("woff2");
 
-    const optimization_guide::ComponentInfo& component_info =
-        test_component_creator_.CreateComponentInfoWithExperimentalPageHints(
-            optimization_guide::proto::RESOURCE_LOADING, hints_sites,
-            resource_patterns);
-
-    g_browser_process->optimization_guide_service()->ProcessHints(
-        component_info);
-
-    // Wait for hints to be processed by PreviewsOptimizationGuide.
-    base::RunLoop().RunUntilIdle();
+    ProcessHintsComponent(
+        test_hints_component_creator_
+            .CreateHintsComponentInfoWithExperimentalPageHints(
+                optimization_guide::proto::RESOURCE_LOADING, hints_sites,
+                resource_patterns));
   }
 
   // Sets the resource loading hints in optimization guide service. Some hints
@@ -192,21 +173,10 @@ class ResourceLoadingNoFeaturesBrowserTest : public InProcessBrowserTest {
     default_resource_patterns.push_back("bar.jpg");
     default_resource_patterns.push_back("woff2");
 
-    const optimization_guide::ComponentInfo& component_info =
-        test_component_creator_.CreateComponentInfoWithMixPageHints(
+    ProcessHintsComponent(
+        test_hints_component_creator_.CreateHintsComponentInfoWithMixPageHints(
             optimization_guide::proto::RESOURCE_LOADING, hints_sites,
-            experimental_resource_patterns, default_resource_patterns);
-
-    g_browser_process->optimization_guide_service()->ProcessHints(
-        component_info);
-
-    // Wait for hints to be processed by PreviewsOptimizationGuide.
-    base::RunLoop().RunUntilIdle();
-  }
-
-  void AddTestOptimizationGuideServiceObserver(
-      TestOptimizationGuideServiceObserver* observer) {
-    g_browser_process->optimization_guide_service()->AddObserver(observer);
+            experimental_resource_patterns, default_resource_patterns));
   }
 
   const GURL& https_url() const { return https_url_; }
@@ -286,7 +256,8 @@ class ResourceLoadingNoFeaturesBrowserTest : public InProcessBrowserTest {
     }
   }
 
-  optimization_guide::testing::TestComponentCreator test_component_creator_;
+  optimization_guide::testing::TestHintsComponentCreator
+      test_hints_component_creator_;
 
   std::unique_ptr<net::EmbeddedTestServer> https_server_;
   std::unique_ptr<net::EmbeddedTestServer> http_server_;
@@ -346,13 +317,8 @@ IN_PROC_BROWSER_TEST_F(
   SetExpectedFooJpgRequest(false);
   SetExpectedBarJpgRequest(true);
 
-  TestOptimizationGuideServiceObserver observer;
-  AddTestOptimizationGuideServiceObserver(&observer);
-  base::RunLoop().RunUntilIdle();
-
   // Whitelist test URL for resource loading hints.
   SetDefaultOnlyResourceLoadingHints({https_url().host()});
-  observer.WaitForNotification();
 
   base::HistogramTester histogram_tester;
 
@@ -410,13 +376,8 @@ IN_PROC_BROWSER_TEST_F(
   SetExpectedFooJpgRequest(true);
   SetExpectedBarJpgRequest(true);
 
-  TestOptimizationGuideServiceObserver observer;
-  AddTestOptimizationGuideServiceObserver(&observer);
-  base::RunLoop().RunUntilIdle();
-
   // Whitelist test URL for resource loading hints.
   SetExperimentOnlyResourceLoadingHints({https_url().host()});
-  observer.WaitForNotification();
 
   base::HistogramTester histogram_tester;
 
@@ -445,13 +406,8 @@ IN_PROC_BROWSER_TEST_F(
   SetExpectedFooJpgRequest(false);
   SetExpectedBarJpgRequest(true);
 
-  TestOptimizationGuideServiceObserver observer;
-  AddTestOptimizationGuideServiceObserver(&observer);
-  base::RunLoop().RunUntilIdle();
-
   // Whitelist test URL for resource loading hints.
   SetExperimentOnlyResourceLoadingHints({https_url().host()});
-  observer.WaitForNotification();
 
   base::HistogramTester histogram_tester;
 
@@ -489,14 +445,9 @@ IN_PROC_BROWSER_TEST_F(
   SetExpectedFooJpgRequest(false);
   SetExpectedBarJpgRequest(true);
 
-  TestOptimizationGuideServiceObserver observer;
-  AddTestOptimizationGuideServiceObserver(&observer);
-  base::RunLoop().RunUntilIdle();
-
   // Whitelist test URL for resource loading hints. Set both experimental and
   // non-experimental hints.
   SetMixResourceLoadingHints({https_url().host()});
-  observer.WaitForNotification();
 
   base::HistogramTester histogram_tester;
 
@@ -534,13 +485,8 @@ IN_PROC_BROWSER_TEST_F(
   SetExpectedFooJpgRequest(true);
   SetExpectedBarJpgRequest(false);
 
-  TestOptimizationGuideServiceObserver observer;
-  AddTestOptimizationGuideServiceObserver(&observer);
-  base::RunLoop().RunUntilIdle();
-
   // Whitelist test URL for resource loading hints.
   SetMixResourceLoadingHints({https_url().host()});
-  observer.WaitForNotification();
 
   base::HistogramTester histogram_tester;
 
@@ -565,12 +511,7 @@ IN_PROC_BROWSER_TEST_F(
   SetExpectedFooJpgRequest(false);
   SetExpectedBarJpgRequest(true);
 
-  TestOptimizationGuideServiceObserver observer;
-  AddTestOptimizationGuideServiceObserver(&observer);
-  base::RunLoop().RunUntilIdle();
-
   SetDefaultOnlyResourceLoadingHints({https_url().host()});
-  observer.WaitForNotification();
 
   base::HistogramTester histogram_tester;
 
@@ -601,12 +542,7 @@ IN_PROC_BROWSER_TEST_F(
   SetExpectedFooJpgRequest(true);
   SetExpectedBarJpgRequest(true);
 
-  TestOptimizationGuideServiceObserver observer;
-  AddTestOptimizationGuideServiceObserver(&observer);
-  base::RunLoop().RunUntilIdle();
-
   SetDefaultOnlyResourceLoadingHints({});
-  observer.WaitForNotification();
 
   base::HistogramTester histogram_tester;
 
@@ -631,13 +567,8 @@ IN_PROC_BROWSER_TEST_F(ResourceLoadingHintsBrowserTest,
   SetExpectedFooJpgRequest(true);
   SetExpectedBarJpgRequest(true);
 
-  TestOptimizationGuideServiceObserver observer;
-  AddTestOptimizationGuideServiceObserver(&observer);
-  base::RunLoop().RunUntilIdle();
-
   // Whitelist test HTTP URL for resource loading hints.
   SetDefaultOnlyResourceLoadingHints({https_url().host()});
-  observer.WaitForNotification();
 
   base::HistogramTester histogram_tester;
 
@@ -660,13 +591,8 @@ IN_PROC_BROWSER_TEST_F(
   SetExpectedFooJpgRequest(true);
   SetExpectedBarJpgRequest(true);
 
-  TestOptimizationGuideServiceObserver observer;
-  AddTestOptimizationGuideServiceObserver(&observer);
-  base::RunLoop().RunUntilIdle();
-
   // Whitelist test URL for resource loading hints.
   SetDefaultOnlyResourceLoadingHints({https_url().host()});
-  observer.WaitForNotification();
 
   base::HistogramTester histogram_tester;
 

components/optimization_guide/BUILD.gn

@@ -4,6 +4,9 @@
 
 static_library("optimization_guide") {
   sources = [
+    "hints_component_info.h",
+    "hints_component_util.cc",
+    "hints_component_util.h",
     "optimization_guide_constants.cc",
     "optimization_guide_constants.h",
     "optimization_guide_service.cc",
@@ -22,8 +25,8 @@ static_library("optimization_guide") {
 static_library("test_support") {
   testonly = true
   sources = [
-    "test_component_creator.cc",
-    "test_component_creator.h",
+    "test_hints_component_creator.cc",
+    "test_hints_component_creator.h",
   ]
   deps = [
     ":optimization_guide",
@@ -36,6 +39,7 @@ static_library("test_support") {
 source_set("unit_tests") {
   testonly = true
   sources = [
+    "hints_component_util_unittest.cc",
     "optimization_guide_service_unittest.cc",
     "url_pattern_with_wildcards_unittest.cc",
   ]

components/optimization_guide/hints_component_info.h

+// Copyright 2018 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_OPTIMIZATION_GUIDE_HINTS_COMPONENT_INFO_H_
+#define COMPONENTS_OPTIMIZATION_GUIDE_HINTS_COMPONENT_INFO_H_
+
+#include "base/files/file_path.h"
+#include "base/version.h"
+
+namespace optimization_guide {
+
+// Information about a version of optimization hints data received from the
+// components server.
+struct HintsComponentInfo {
+  HintsComponentInfo(const base::Version& version, const base::FilePath& path)
+      : version(version), path(path) {}
+
+  // The version of the hints content.
+  const base::Version version;
+
+  // The path to the file containing the hints protobuf file.
+  const base::FilePath path;
+};
+
+}  // namespace optimization_guide
+
+#endif  // COMPONENTS_OPTIMIZATION_GUIDE_HINTS_COMPONENT_INFO_H_

components/optimization_guide/hints_component_util.cc

+// Copyright 2018 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/optimization_guide/hints_component_util.h"
+
+#include <string>
+
+#include "base/files/file.h"
+#include "base/files/file_util.h"
+#include "base/metrics/histogram_macros.h"
+#include "components/optimization_guide/hints_component_info.h"
+#include "components/optimization_guide/proto/hints.pb.h"
+
+namespace optimization_guide {
+
+namespace {
+
+void RecordProcessHintsComponentResult(ProcessHintsComponentResult result) {
+  UMA_HISTOGRAM_ENUMERATION(kProcessHintsComponentResultHistogramString,
+                            static_cast<int>(result),
+                            static_cast<int>(ProcessHintsComponentResult::MAX));
+}
+
+}  // namespace
+
+const char kProcessHintsComponentResultHistogramString[] =
+    "OptimizationGuide.ProcessHintsResult";
+
+std::unique_ptr<proto::Configuration> ProcessHintsComponent(
+    const HintsComponentInfo& component_info) {
+  if (!component_info.version.IsValid() || component_info.path.empty()) {
+    RecordProcessHintsComponentResult(
+        ProcessHintsComponentResult::FAILED_INVALID_PARAMETERS);
+    return nullptr;
+  }
+
+  std::string binary_pb;
+  if (!base::ReadFileToString(component_info.path, &binary_pb)) {
+    RecordProcessHintsComponentResult(
+        ProcessHintsComponentResult::FAILED_READING_FILE);
+    return nullptr;
+  }
+
+  std::unique_ptr<proto::Configuration> proto_configuration =
+      std::make_unique<proto::Configuration>();
+  if (!proto_configuration->ParseFromString(binary_pb)) {
+    RecordProcessHintsComponentResult(
+        ProcessHintsComponentResult::FAILED_INVALID_CONFIGURATION);
+    return nullptr;
+  }
+
+  RecordProcessHintsComponentResult(ProcessHintsComponentResult::SUCCESS);
+  return proto_configuration;
+}
+
+}  // namespace optimization_guide

components/optimization_guide/hints_component_util.h

+// Copyright 2018 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_OPTIMIZATION_GUIDE_HINTS_COMPONENT_UTIL_H_
+#define COMPONENTS_OPTIMIZATION_GUIDE_HINTS_COMPONENT_UTIL_H_
+
+#include <memory>
+
+namespace optimization_guide {
+
+struct HintsComponentInfo;
+namespace proto {
+class Configuration;
+}  // namespace proto
+
+// The UMA histogram used to record the result of processing the hints
+// component.
+extern const char kProcessHintsComponentResultHistogramString[];
+
+// Enumerates the possible outcomes of processing the hints component. Used in
+// UMA histograms, so the order of enumerators should not be changed.
+//
+// Keep in sync with OptimizationGuideProcessHintsResult in
+// tools/metrics/histograms/enums.xml.
+enum class ProcessHintsComponentResult {
+  SUCCESS,
+  FAILED_INVALID_PARAMETERS,
+  FAILED_READING_FILE,
+  FAILED_INVALID_CONFIGURATION,
+
+  // Insert new values before this line.
+  MAX,
+};
+
+// Processes the specified hints component, records the result in a UMA
+// histogram, and, if successful, returns the component's Configuration
+// protobuf. If unsuccessful, returns a nullptr.
+std::unique_ptr<proto::Configuration> ProcessHintsComponent(
+    const HintsComponentInfo& info);
+
+}  // namespace optimization_guide
+
+#endif  // COMPONENTS_OPTIMIZATION_GUIDE_HINTS_COMPONENT_UTIL_H_

components/optimization_guide/hints_component_util_unittest.cc

+// Copyright 2018 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/optimization_guide/hints_component_util.h"
+
+#include <string>
+
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/macros.h"
+#include "base/test/metrics/histogram_tester.h"
+#include "base/version.h"
+#include "components/optimization_guide/hints_component_info.h"
+#include "components/optimization_guide/proto/hints.pb.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace optimization_guide {
+
+const base::FilePath::CharType kFileName[] = FILE_PATH_LITERAL("somefile.pb");
+
+class HintsComponentUtilTest : public testing::Test {
+ public:
+  HintsComponentUtilTest() {}
+
+  ~HintsComponentUtilTest() override {}
+
+  void SetUp() override { ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); }
+
+  void WriteConfigToFile(const base::FilePath& filePath,
+                         const proto::Configuration& config) {
+    std::string serialized_config;
+    ASSERT_TRUE(config.SerializeToString(&serialized_config));
+    ASSERT_EQ(static_cast<int32_t>(serialized_config.length()),
+              base::WriteFile(filePath, serialized_config.data(),
+                              serialized_config.length()));
+  }
+
+  base::FilePath temp_dir() const { return temp_dir_.GetPath(); }
+
+ private:
+  base::ScopedTempDir temp_dir_;
+
+  DISALLOW_COPY_AND_ASSIGN(HintsComponentUtilTest);
+};
+
+TEST_F(HintsComponentUtilTest, ProcessHintsComponentInvalidVersion) {
+  base::HistogramTester histogram_tester;
+  std::unique_ptr<proto::Configuration> config = ProcessHintsComponent(
+      HintsComponentInfo(base::Version(""), base::FilePath(kFileName)));
+
+  EXPECT_FALSE(config);
+  histogram_tester.ExpectUniqueSample(
+      kProcessHintsComponentResultHistogramString,
+      static_cast<int>(ProcessHintsComponentResult::FAILED_INVALID_PARAMETERS),
+      1);
+}
+
+TEST_F(HintsComponentUtilTest, ProcessHintsComponentInvalidPath) {
+  base::HistogramTester histogram_tester;
+  std::unique_ptr<proto::Configuration> config = ProcessHintsComponent(
+      HintsComponentInfo(base::Version("1.0.0.0"), base::FilePath()));
+
+  EXPECT_FALSE(config);
+  histogram_tester.ExpectUniqueSample(
+      kProcessHintsComponentResultHistogramString,
+      static_cast<int>(ProcessHintsComponentResult::FAILED_INVALID_PARAMETERS),
+      1);
+}
+
+TEST_F(HintsComponentUtilTest, ProcessHintsComponentInvalidFile) {
+  base::HistogramTester histogram_tester;
+  std::unique_ptr<proto::Configuration> config = ProcessHintsComponent(
+      HintsComponentInfo(base::Version("1.0.0"), base::FilePath(kFileName)));
+
+  EXPECT_FALSE(config);
+  histogram_tester.ExpectUniqueSample(
+      kProcessHintsComponentResultHistogramString,
+      static_cast<int>(ProcessHintsComponentResult::FAILED_READING_FILE), 1);
+}
+
+TEST_F(HintsComponentUtilTest, ProcessHintsComponentNotAConfigInFile) {
+  base::HistogramTester histogram_tester;
+
+  const base::FilePath filePath = temp_dir().Append(kFileName);
+  ASSERT_EQ(static_cast<int32_t>(3), base::WriteFile(filePath, "boo", 3));
+
+  std::unique_ptr<proto::Configuration> config = ProcessHintsComponent(
+      HintsComponentInfo(base::Version("1.0.0"), filePath));
+
+  EXPECT_FALSE(config);
+  histogram_tester.ExpectUniqueSample(
+      kProcessHintsComponentResultHistogramString,
+      static_cast<int>(
+          ProcessHintsComponentResult::FAILED_INVALID_CONFIGURATION),
+      1);
+}
+
+TEST_F(HintsComponentUtilTest, ProcessHintsComponentSuccess) {
+  base::HistogramTester histogram_tester;
+
+  const base::FilePath filePath = temp_dir().Append(kFileName);
+  proto::Configuration config;
+  proto::Hint* hint = config.add_hints();
+  hint->set_key("google.com");
+  ASSERT_NO_FATAL_FAILURE(WriteConfigToFile(filePath, config));
+
+  std::unique_ptr<proto::Configuration> processed_config =
+      ProcessHintsComponent(
+          HintsComponentInfo(base::Version("1.0.0"), filePath));
+
+  ASSERT_TRUE(processed_config);
+  EXPECT_EQ(1, processed_config->hints_size());
+  EXPECT_EQ("google.com", processed_config->hints()[0].key());
+  histogram_tester.ExpectUniqueSample(
+      kProcessHintsComponentResultHistogramString,
+      static_cast<int>(ProcessHintsComponentResult::SUCCESS), 1);
+}
+
+}  // namespace optimization_guide

components/optimization_guide/optimization_guide_service.cc

@@ -4,59 +4,26 @@
 
 #include "components/optimization_guide/optimization_guide_service.h"
 
-#include <string>
-
 #include "base/bind.h"
-#include "base/files/file.h"
-#include "base/files/file_util.h"
-#include "base/metrics/histogram_macros.h"
 #include "base/task/post_task.h"
 
 namespace optimization_guide {
 
-namespace {
-
-// Version "0" corresponds to no processed version. By service conventions,
-// we represent it as a dotted triple.
-const char kNullVersion[] = "0.0.0";
-
-void RecordProcessHintsResult(
-    OptimizationGuideService::ProcessHintsResult result) {
-  UMA_HISTOGRAM_ENUMERATION(
-      "OptimizationGuide.ProcessHintsResult", static_cast<int>(result),
-      static_cast<int>(OptimizationGuideService::ProcessHintsResult::MAX));
-}
-
-}  // namespace
-
-ComponentInfo::ComponentInfo(const base::Version& hints_version,
-                             const base::FilePath& hints_path)
-    : hints_version(hints_version), hints_path(hints_path) {}
-
-ComponentInfo::~ComponentInfo() {}
-
 OptimizationGuideService::OptimizationGuideService(
     const scoped_refptr<base::SingleThreadTaskRunner>& ui_thread_task_runner)
-    : background_task_runner_(base::CreateSequencedTaskRunnerWithTraits(
-          {base::MayBlock(), base::TaskPriority::BEST_EFFORT})),
-      ui_thread_task_runner_(ui_thread_task_runner),
-      latest_processed_version_(kNullVersion) {
+    : ui_thread_task_runner_(ui_thread_task_runner), weak_ptr_factory_(this) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 }
 
 OptimizationGuideService::~OptimizationGuideService() {}
 
-void OptimizationGuideService::SetLatestProcessedVersionForTesting(
-    const base::Version& version) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  latest_processed_version_ = version;
-}
-
 void OptimizationGuideService::AddObserver(
     OptimizationGuideServiceObserver* observer) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-
   observers_.AddObserver(observer);
+  if (hints_component_info_) {
+    observer->OnHintsComponentAvailable(*hints_component_info_);
+  }
 }
 
 void OptimizationGuideService::RemoveObserver(
@@ -65,55 +32,33 @@ void OptimizationGuideService::RemoveObserver(
   observers_.RemoveObserver(observer);
 }
 
-void OptimizationGuideService::ProcessHints(
-    const ComponentInfo& component_info) {
-  background_task_runner_->PostTask(
+void OptimizationGuideService::MaybeUpdateHintsComponent(
+    const HintsComponentInfo& info) {
+  ui_thread_task_runner_->PostTask(
       FROM_HERE,
-      base::BindOnce(&OptimizationGuideService::ProcessHintsInBackground,
-                     base::Unretained(this), component_info));
+      base::BindOnce(
+          &OptimizationGuideService::MaybeUpdateHintsComponentOnUIThread,
+          weak_ptr_factory_.GetWeakPtr(), info));
 }
 
-void OptimizationGuideService::ProcessHintsInBackground(
-    const ComponentInfo& component_info) {
-  DCHECK(background_task_runner_->RunsTasksInCurrentSequence());
-
-  if (!component_info.hints_version.IsValid()) {
-    RecordProcessHintsResult(ProcessHintsResult::FAILED_INVALID_PARAMETERS);
-    return;
-  }
-  if (latest_processed_version_.CompareTo(component_info.hints_version) >= 0)
-    return;
-  if (component_info.hints_path.empty()) {
-    RecordProcessHintsResult(ProcessHintsResult::FAILED_INVALID_PARAMETERS);
-    return;
-  }
-  std::string binary_pb;
-  if (!base::ReadFileToString(component_info.hints_path, &binary_pb)) {
-    RecordProcessHintsResult(ProcessHintsResult::FAILED_READING_FILE);
+void OptimizationGuideService::MaybeUpdateHintsComponentOnUIThread(
+    const HintsComponentInfo& info) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK(info.version.IsValid());
+  DCHECK(!info.path.empty());
+
+  // Do not update the component if the version isn't newer. This differs from
+  // the check in ComponentInstaller::InstallHelper(), because this rejects
+  // version equality, whereas InstallHelper() accepts it.
+  if (hints_component_info_ &&
+      hints_component_info_->version.CompareTo(info.version) >= 0) {
     return;
   }
 
-  proto::Configuration new_config;
-  if (!new_config.ParseFromString(binary_pb)) {
-    RecordProcessHintsResult(ProcessHintsResult::FAILED_INVALID_CONFIGURATION);
-    return;
+  hints_component_info_.emplace(info.version, info.path);
+  for (auto& observer : observers_) {
+    observer.OnHintsComponentAvailable(*hints_component_info_);
   }
-  latest_processed_version_ = component_info.hints_version;
-
-  RecordProcessHintsResult(ProcessHintsResult::SUCCESS);
-  ui_thread_task_runner_->PostTask(
-      FROM_HERE,
-      base::BindOnce(&OptimizationGuideService::DispatchHintsOnUIThread,
-                     base::Unretained(this), new_config, component_info));
-}
-
-void OptimizationGuideService::DispatchHintsOnUIThread(
-    const proto::Configuration& config,
-    const ComponentInfo& component_info) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-
-  for (auto& observer : observers_)
-    observer.OnHintsProcessed(config, component_info);
 }
 
 }  // namespace optimization_guide

components/optimization_guide/optimization_guide_service.h

@@ -5,69 +5,47 @@
 #ifndef COMPONENTS_OPTIMIZATION_GUIDE_OPTIMIZATION_GUIDE_SERVICE_H_
 #define COMPONENTS_OPTIMIZATION_GUIDE_OPTIMIZATION_GUIDE_SERVICE_H_
 
-#include <memory>
-
-#include "base/files/file_path.h"
 #include "base/macros.h"
-#include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
+#include "base/optional.h"
 #include "base/sequence_checker.h"
-#include "base/sequenced_task_runner.h"
 #include "base/single_thread_task_runner.h"
-#include "base/version.h"
+#include "components/optimization_guide/hints_component_info.h"
 #include "components/optimization_guide/optimization_guide_service_observer.h"
-#include "components/optimization_guide/proto/hints.pb.h"
 
 namespace optimization_guide {
 
-// Processes the hints downloaded from the Component Updater as part of the
-// Optimization Hints component.
+// Tracks the info for the current Optimization Hints component and notifies
+// observers of newly available hints components downloaded from the Component
+// Updater.
 class OptimizationGuideService {
  public:
-  // Enumerates the possible outcomes of processing hints. Used in UMA
-  // histograms, so the order of enumerators should not be changed.
-  //
-  // Keep in sync with OptimizationGuideProcessHintsResult in
-  // tools/metrics/histograms/enums.xml.
-  enum class ProcessHintsResult {
-    SUCCESS,
-    FAILED_INVALID_PARAMETERS,
-    FAILED_READING_FILE,
-    FAILED_INVALID_CONFIGURATION,
-
-    // Insert new values before this line.
-    MAX,
-  };
-
   explicit OptimizationGuideService(
       const scoped_refptr<base::SingleThreadTaskRunner>& ui_thread_task_runner);
 
   virtual ~OptimizationGuideService();
 
+  // Adds the observer and synchronously dispatches the current
+  // HintsComponentInfo to it if one is already available.
   void AddObserver(OptimizationGuideServiceObserver* observer);
   // Virtual so it can be mocked out in tests.
   virtual void RemoveObserver(OptimizationGuideServiceObserver* observer);
 
-  // Processes hints from the given unindexed hints, unless its |hints_version|
-  // matches that of the most recently parsed version, in which case it does
-  // nothing.
+  // Forwards the update hints component request on to the UI thread, where the
+  // actual work occurs.
   //
   // Virtual so it can be mocked out in tests.
-  virtual void ProcessHints(const ComponentInfo& component_info);
-
-  // Sets the latest processed version for testing.
-  void SetLatestProcessedVersionForTesting(const base::Version& version);
+  virtual void MaybeUpdateHintsComponent(const HintsComponentInfo& info);
 
  private:
-  // Always called as part of a BEST_EFFORT priority task.
-  void ProcessHintsInBackground(const ComponentInfo& component_info);
-
-  // Dispatches hints to listeners on UI thread.
-  void DispatchHintsOnUIThread(const proto::Configuration& config,
-                               const ComponentInfo& component_info);
+  // If the hints component version in |info| is greater than that in
+  // |hints_component_info_|, updates |hints_component_info_| and dispatches it
+  // to all observers. In the case where the version is not greater, it does
+  // nothing.
+  void MaybeUpdateHintsComponentOnUIThread(const HintsComponentInfo& info);
 
   // Runner for indexing tasks.
-  scoped_refptr<base::SequencedTaskRunner> background_task_runner_;
   SEQUENCE_CHECKER(sequence_checker_);
 
   // Runner for UI Thread tasks.
@@ -76,7 +54,12 @@ class OptimizationGuideService {
   // Observers receiving notifications on hints being processed.
   base::ObserverList<OptimizationGuideServiceObserver>::Unchecked observers_;
 
-  base::Version latest_processed_version_;
+  // The current HintsComponentInfo available to observers. This is unset until
+  // the first time MaybeUpdateHintsComponent() is called.
+  base::Optional<HintsComponentInfo> hints_component_info_;
+
+  // Used to get |weak_ptr_| to self.
+  base::WeakPtrFactory<OptimizationGuideService> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(OptimizationGuideService);
 };

components/optimization_guide/optimization_guide_service_observer.h

@@ -5,35 +5,20 @@
 #ifndef COMPONENTS_OPTIMIZATION_GUIDE_OPTIMIZATION_GUIDE_SERVICE_OBSERVER_H_
 #define COMPONENTS_OPTIMIZATION_GUIDE_OPTIMIZATION_GUIDE_SERVICE_OBSERVER_H_
 
-#include "base/version.h"
-#include "components/optimization_guide/proto/hints.pb.h"
-
 namespace optimization_guide {
 
-// Encapsulates information about a version of optimization hints data received
-// from the components server.
-struct ComponentInfo {
-  ComponentInfo(const base::Version& hints_version,
-                const base::FilePath& hints_path);
-  ~ComponentInfo();
-
-  // The version of the hints content.
-  const base::Version hints_version;
-
-  // The path to the file containing the hints protobuf file.
-  const base::FilePath hints_path;
-};
+struct HintsComponentInfo;
 
 // Interface for objects that wish to be notified of changes in the Optimization
 // Guide Service.
 //
-// All calls will be made on the IO thread.
+// All calls will be made on the UI thread.
 class OptimizationGuideServiceObserver {
  public:
-  // Called when the hints have been processed.
-  virtual void OnHintsProcessed(
-      const proto::Configuration& config,
-      const optimization_guide::ComponentInfo& component_info) = 0;
+  // Called when a new hints component is available for processing. While this
+  // is called on the UI thread, it is recommended that processing of the new
+  // component via ProcessHintsComponent() occur on a background thread.
+  virtual void OnHintsComponentAvailable(const HintsComponentInfo& info) = 0;
 
  protected:
   virtual ~OptimizationGuideServiceObserver() {}

components/optimization_guide/optimization_guide_service_unittest.cc

@@ -5,47 +5,51 @@
 #include "components/optimization_guide/optimization_guide_service.h"
 
 #include <memory>
+#include <string>
 
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "base/run_loop.h"
-#include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_task_environment.h"
 #include "base/version.h"
-#include "components/optimization_guide/optimization_guide_service_observer.h"
-#include "components/optimization_guide/proto/hints.pb.h"
+#include "components/optimization_guide/hints_component_info.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace optimization_guide {
 
-const base::FilePath::CharType kFileName[] = FILE_PATH_LITERAL("somefile.pb");
+const base::FilePath::CharType kFileName1[] = FILE_PATH_LITERAL("somefile1.pb");
+const base::FilePath::CharType kFileName2[] = FILE_PATH_LITERAL("somefile2.pb");
 
 class TestObserver : public OptimizationGuideServiceObserver {
  public:
-  TestObserver() : received_notification_(false) {}
+  TestObserver()
+      : hints_component_notification_count_(0),
+        hints_component_version_("0.0.0.0") {}
 
   ~TestObserver() override {}
 
-  void OnHintsProcessed(
-      const proto::Configuration& config,
-      const optimization_guide::ComponentInfo& component_info) override {
-    received_notification_ = true;
-    received_config_ = config;
-    received_version_ = component_info.hints_version;
+  void OnHintsComponentAvailable(const HintsComponentInfo& info) override {
+    ++hints_component_notification_count_;
+    hints_component_version_ = info.version;
+    hints_component_path_ = info.path;
   }
 
-  bool received_notification() const { return received_notification_; }
-
-  proto::Configuration received_config() const { return received_config_; }
-
-  base::Version received_version() const { return received_version_; }
+  int hints_component_notification_count() const {
+    return hints_component_notification_count_;
+  }
+  const base::Version& hints_component_version() const {
+    return hints_component_version_;
+  }
+  const base::FilePath& hints_component_path() const {
+    return hints_component_path_;
+  }
 
  private:
-  bool received_notification_;
-  proto::Configuration received_config_;
-  base::Version received_version_;
+  int hints_component_notification_count_;
+  base::Version hints_component_version_;
+  base::FilePath hints_component_path_;
 
   DISALLOW_COPY_AND_ASSIGN(TestObserver);
 };
@@ -76,29 +80,14 @@ class OptimizationGuideServiceTest : public testing::Test {
     optimization_guide_service_->RemoveObserver(observer());
   }
 
-  void UpdateHints(const base::Version& version,
-                   const base::FilePath& filePath) {
-    ComponentInfo info(version, filePath);
-    optimization_guide_service_->ProcessHints(info);
-  }
-
-  void WriteConfigToFile(const base::FilePath& filePath,
-                         const proto::Configuration& config) {
-    std::string serialized_config;
-    ASSERT_TRUE(config.SerializeToString(&serialized_config));
-    ASSERT_EQ(static_cast<int32_t>(serialized_config.length()),
-              base::WriteFile(filePath, serialized_config.data(),
-                              serialized_config.length()));
-  }
-
-  base::FilePath temp_dir() const { return temp_dir_.GetPath(); }
-
- protected:
-  void RunUntilIdle() {
+  void MaybeUpdateHintsComponent(const HintsComponentInfo& info) {
+    optimization_guide_service_->MaybeUpdateHintsComponent(info);
     scoped_task_environment_.RunUntilIdle();
     base::RunLoop().RunUntilIdle();
   }
 
+  base::FilePath temp_dir() const { return temp_dir_.GetPath(); }
+
  private:
   base::test::ScopedTaskEnvironment scoped_task_environment_;
   base::ScopedTempDir temp_dir_;
@@ -109,128 +98,65 @@ class OptimizationGuideServiceTest : public testing::Test {
   DISALLOW_COPY_AND_ASSIGN(OptimizationGuideServiceTest);
 };
 
-TEST_F(OptimizationGuideServiceTest, ProcessHintsInvalidVersionIgnored) {
-  base::HistogramTester histogram_tester;
-  AddObserver();
-  UpdateHints(base::Version(""), base::FilePath(kFileName));
-
-  RunUntilIdle();
-
-  EXPECT_FALSE(observer()->received_notification());
-  histogram_tester.ExpectUniqueSample(
-      "OptimizationGuide.ProcessHintsResult",
-      static_cast<int>(OptimizationGuideService::ProcessHintsResult::
-                           FAILED_INVALID_PARAMETERS),
-      1);
-}
-
-TEST_F(OptimizationGuideServiceTest, ProcessHintsPastVersionIgnored) {
+TEST_F(OptimizationGuideServiceTest, ProcessHintsIssuesNotification) {
   AddObserver();
-  optimization_guide_service()->SetLatestProcessedVersionForTesting(
-      base::Version("2.0.0"));
 
-  const base::FilePath filePath = temp_dir().Append(kFileName);
-  proto::Configuration config;
-  proto::Hint* hint = config.add_hints();
-  hint->set_key("google.com");
-  ASSERT_NO_FATAL_FAILURE(WriteConfigToFile(filePath, config));
+  HintsComponentInfo component_info(base::Version("1.0.0.0"),
+                                    temp_dir().Append(kFileName1));
 
-  UpdateHints(base::Version("1.0.0"), filePath);
+  MaybeUpdateHintsComponent(component_info);
 
-  RunUntilIdle();
-
-  EXPECT_FALSE(observer()->received_notification());
+  EXPECT_EQ(observer()->hints_component_notification_count(), 1);
+  EXPECT_EQ(component_info.version, observer()->hints_component_version());
+  EXPECT_EQ(component_info.path, observer()->hints_component_path());
 }
 
-TEST_F(OptimizationGuideServiceTest, ProcessHintsSameVersionIgnored) {
+TEST_F(OptimizationGuideServiceTest, ProcessHintsNewVersionProcessed) {
   AddObserver();
-  const base::Version version("1.0.0");
-  optimization_guide_service()->SetLatestProcessedVersionForTesting(version);
-
-  const base::FilePath filePath = temp_dir().Append(kFileName);
-  proto::Configuration config;
-  proto::Hint* hint = config.add_hints();
-  hint->set_key("google.com");
-  ASSERT_NO_FATAL_FAILURE(WriteConfigToFile(filePath, config));
 
-  UpdateHints(version, filePath);
+  HintsComponentInfo component_info_1(base::Version("1.0.0.0"),
+                                      temp_dir().Append(kFileName1));
+  HintsComponentInfo component_info_2(base::Version("2.0.0.0"),
+                                      temp_dir().Append(kFileName2));
 
-  RunUntilIdle();
+  MaybeUpdateHintsComponent(component_info_1);
+  MaybeUpdateHintsComponent(component_info_2);
 
-  EXPECT_FALSE(observer()->received_notification());
+  EXPECT_EQ(observer()->hints_component_notification_count(), 2);
+  EXPECT_EQ(component_info_2.version, observer()->hints_component_version());
+  EXPECT_EQ(component_info_2.path, observer()->hints_component_path());
 }
 
-TEST_F(OptimizationGuideServiceTest, ProcessHintsEmptyFileNameIgnored) {
-  base::HistogramTester histogram_tester;
+TEST_F(OptimizationGuideServiceTest, ProcessHintsPastVersionIgnored) {
   AddObserver();
-  UpdateHints(base::Version("1.0.0"), base::FilePath(FILE_PATH_LITERAL("")));
-
-  RunUntilIdle();
 
-  EXPECT_FALSE(observer()->received_notification());
-  histogram_tester.ExpectUniqueSample(
-      "OptimizationGuide.ProcessHintsResult",
-      static_cast<int>(OptimizationGuideService::ProcessHintsResult::
-                           FAILED_INVALID_PARAMETERS),
-      1);
-}
-
-TEST_F(OptimizationGuideServiceTest, ProcessHintsInvalidFileIgnored) {
-  base::HistogramTester histogram_tester;
-  AddObserver();
-  UpdateHints(base::Version("1.0.0"), base::FilePath(kFileName));
+  HintsComponentInfo component_info_1(base::Version("2.0.0.0"),
+                                      temp_dir().Append(kFileName1));
+  HintsComponentInfo component_info_2(base::Version("1.0.0.0"),
+                                      temp_dir().Append(kFileName2));
 
-  RunUntilIdle();
+  MaybeUpdateHintsComponent(component_info_1);
+  MaybeUpdateHintsComponent(component_info_2);
 
-  EXPECT_FALSE(observer()->received_notification());
-  histogram_tester.ExpectUniqueSample(
-      "OptimizationGuide.ProcessHintsResult",
-      static_cast<int>(
-          OptimizationGuideService::ProcessHintsResult::FAILED_READING_FILE),
-      1);
+  EXPECT_EQ(observer()->hints_component_notification_count(), 1);
+  EXPECT_EQ(component_info_1.version, observer()->hints_component_version());
+  EXPECT_EQ(component_info_1.path, observer()->hints_component_path());
 }
 
-TEST_F(OptimizationGuideServiceTest, ProcessHintsNotAConfigInFileIgnored) {
-  base::HistogramTester histogram_tester;
+TEST_F(OptimizationGuideServiceTest, ProcessHintsSameVersionIgnored) {
   AddObserver();
-  const base::FilePath filePath = temp_dir().Append(kFileName);
-  ASSERT_EQ(static_cast<int32_t>(3), base::WriteFile(filePath, "boo", 3));
 
-  UpdateHints(base::Version("1.0.0"), filePath);
+  HintsComponentInfo component_info_1(base::Version("2.0.0.0"),
+                                      temp_dir().Append(kFileName1));
+  HintsComponentInfo component_info_2(base::Version("2.0.0.0"),
+                                      temp_dir().Append(kFileName2));
 
-  RunUntilIdle();
+  MaybeUpdateHintsComponent(component_info_1);
+  MaybeUpdateHintsComponent(component_info_2);
 
-  EXPECT_FALSE(observer()->received_notification());
-  histogram_tester.ExpectUniqueSample(
-      "OptimizationGuide.ProcessHintsResult",
-      static_cast<int>(OptimizationGuideService::ProcessHintsResult::
-                           FAILED_INVALID_CONFIGURATION),
-      1);
-}
-
-TEST_F(OptimizationGuideServiceTest, ProcessHintsIssuesNotification) {
-  base::HistogramTester histogram_tester;
-  AddObserver();
-  const base::FilePath filePath = temp_dir().Append(kFileName);
-  proto::Configuration config;
-  proto::Hint* hint = config.add_hints();
-  hint->set_key("google.com");
-  ASSERT_NO_FATAL_FAILURE(WriteConfigToFile(filePath, config));
-
-  base::Version hints_version("1.0.0");
-  UpdateHints(hints_version, filePath);
-
-  RunUntilIdle();
-
-  EXPECT_TRUE(observer()->received_notification());
-  proto::Configuration received_config = observer()->received_config();
-  ASSERT_EQ(1, received_config.hints_size());
-  ASSERT_EQ("google.com", received_config.hints()[0].key());
-  EXPECT_EQ(0, observer()->received_version().CompareTo(hints_version));
-  histogram_tester.ExpectUniqueSample(
-      "OptimizationGuide.ProcessHintsResult",
-      static_cast<int>(OptimizationGuideService::ProcessHintsResult::SUCCESS),
-      1);
+  EXPECT_EQ(observer()->hints_component_notification_count(), 1);
+  EXPECT_EQ(component_info_1.version, observer()->hints_component_version());
+  EXPECT_EQ(component_info_1.path, observer()->hints_component_path());
 }
 
 TEST_F(OptimizationGuideServiceTest,
@@ -239,17 +165,26 @@ TEST_F(OptimizationGuideServiceTest,
   AddObserver();
   RemoveObserver();
 
-  const base::FilePath filePath = temp_dir().Append(kFileName);
-  proto::Configuration config;
-  proto::Hint* hint = config.add_hints();
-  hint->set_key("google.com");
-  ASSERT_NO_FATAL_FAILURE(WriteConfigToFile(filePath, config));
+  HintsComponentInfo component_info(base::Version("1.0.0.0"),
+                                    temp_dir().Append(kFileName1));
 
-  UpdateHints(base::Version("1.0.0"), filePath);
+  MaybeUpdateHintsComponent(component_info);
+
+  EXPECT_EQ(observer()->hints_component_notification_count(), 0);
+}
 
-  RunUntilIdle();
+TEST_F(OptimizationGuideServiceTest,
+       RegisteredObserverReceivesNotificationForCurrentComponent) {
+  HintsComponentInfo component_info(base::Version("1.0.0.0"),
+                                    temp_dir().Append(kFileName1));
+
+  MaybeUpdateHintsComponent(component_info);
+
+  AddObserver();
 
-  EXPECT_FALSE(observer()->received_notification());
+  EXPECT_EQ(observer()->hints_component_notification_count(), 1);
+  EXPECT_EQ(component_info.version, observer()->hints_component_version());
+  EXPECT_EQ(component_info.path, observer()->hints_component_path());
 }
 
 }  // namespace optimization_guide

components/optimization_guide/test_component_creator.cc → components/optimization_guide/test_hints_component_creator.cc

@@ -2,29 +2,28 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "components/optimization_guide/test_component_creator.h"
+#include "components/optimization_guide/test_hints_component_creator.h"
 
 #include "base/files/file_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/version.h"
-#include "components/optimization_guide/proto/hints.pb.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace optimization_guide {
 namespace testing {
 
-TestComponentCreator::TestComponentCreator()
+TestHintsComponentCreator::TestHintsComponentCreator()
     : scoped_temp_dir_(std::make_unique<base::ScopedTempDir>()),
       next_component_version_(1) {}
 
-TestComponentCreator::~TestComponentCreator() {
+TestHintsComponentCreator::~TestHintsComponentCreator() {
   base::ScopedAllowBlockingForTesting allow_blocking;
   scoped_temp_dir_.reset();
 }
 
-optimization_guide::ComponentInfo
-TestComponentCreator::CreateComponentInfoWithPageHints(
+optimization_guide::HintsComponentInfo
+TestHintsComponentCreator::CreateHintsComponentInfoWithPageHints(
     optimization_guide::proto::OptimizationType optimization_type,
     const std::vector<std::string>& page_hint_host_suffixes,
     const std::vector<std::string>& resource_blocking_patterns) {
@@ -50,11 +49,11 @@ TestComponentCreator::CreateComponentInfoWithPageHints(
     }
   }
 
-  return WriteConfigToFileAndReturnComponentInfo(config);
+  return WriteConfigToFileAndReturnHintsComponentInfo(config);
 }
 
-optimization_guide::ComponentInfo
-TestComponentCreator::CreateComponentInfoWithExperimentalPageHints(
+optimization_guide::HintsComponentInfo
+TestHintsComponentCreator::CreateHintsComponentInfoWithExperimentalPageHints(
     optimization_guide::proto::OptimizationType optimization_type,
     const std::vector<std::string>& page_hint_host_suffixes,
     const std::vector<std::string>& experimental_resource_patterns) {
@@ -81,11 +80,11 @@ TestComponentCreator::CreateComponentInfoWithExperimentalPageHints(
     }
   }
 
-  return WriteConfigToFileAndReturnComponentInfo(config);
+  return WriteConfigToFileAndReturnHintsComponentInfo(config);
 }
 
-optimization_guide::ComponentInfo
-TestComponentCreator::CreateComponentInfoWithMixPageHints(
+optimization_guide::HintsComponentInfo
+TestHintsComponentCreator::CreateHintsComponentInfoWithMixPageHints(
     optimization_guide::proto::OptimizationType optimization_type,
     const std::vector<std::string>& page_hint_host_suffixes,
     const std::vector<std::string>& experimental_resource_patterns,
@@ -130,17 +129,18 @@ TestComponentCreator::CreateComponentInfoWithMixPageHints(
     }
   }
 
-  return WriteConfigToFileAndReturnComponentInfo(config);
+  return WriteConfigToFileAndReturnHintsComponentInfo(config);
 }
 
-base::FilePath TestComponentCreator::GetFilePath(std::string file_path_suffix) {
+base::FilePath TestHintsComponentCreator::GetFilePath(
+    std::string file_path_suffix) {
   base::ScopedAllowBlockingForTesting allow_blocking;
   EXPECT_TRUE(scoped_temp_dir_->IsValid() ||
               scoped_temp_dir_->CreateUniqueTempDir());
   return scoped_temp_dir_->GetPath().AppendASCII(file_path_suffix);
 }
 
-void TestComponentCreator::WriteConfigToFile(
+void TestHintsComponentCreator::WriteConfigToFile(
     const base::FilePath& file_path,
     const optimization_guide::proto::Configuration& config) {
   base::ScopedAllowBlockingForTesting allow_blocking;
@@ -153,14 +153,14 @@ void TestComponentCreator::WriteConfigToFile(
                             serialized_config.length()));
 }
 
-optimization_guide::ComponentInfo
-TestComponentCreator::WriteConfigToFileAndReturnComponentInfo(
+optimization_guide::HintsComponentInfo
+TestHintsComponentCreator::WriteConfigToFileAndReturnHintsComponentInfo(
     const optimization_guide::proto::Configuration& config) {
   std::string version_string = base::IntToString(next_component_version_++);
   base::FilePath file_path = GetFilePath(version_string);
   WriteConfigToFile(file_path, config);
-  return optimization_guide::ComponentInfo(base::Version(version_string),
-                                           file_path);
+  return optimization_guide::HintsComponentInfo(base::Version(version_string),
+                                                file_path);
 }
 
 }  // namespace testing

components/optimization_guide/test_component_creator.h → components/optimization_guide/test_hints_component_creator.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef COMPONENTS_OPTIMIZATION_GUIDE_TEST_COMPONENT_CREATOR_H_
-#define COMPONENTS_OPTIMIZATION_GUIDE_TEST_COMPONENT_CREATOR_H_
+#ifndef COMPONENTS_OPTIMIZATION_GUIDE_TEST_HINTS_COMPONENT_CREATOR_H_
+#define COMPONENTS_OPTIMIZATION_GUIDE_TEST_HINTS_COMPONENT_CREATOR_H_
 
 #include <string>
 #include <vector>
@@ -12,6 +12,7 @@
 #include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "components/optimization_guide/optimization_guide_service.h"
+#include "components/optimization_guide/proto/hints.pb.h"
 
 namespace optimization_guide {
 namespace testing {
@@ -23,31 +24,31 @@ static const char kFooExperimentName[] = "foo_experiment";
 //
 // All temporary files and paths are cleaned up when this instance goes out of
 // scope.
-class TestComponentCreator {
+class TestHintsComponentCreator {
  public:
-  TestComponentCreator();
-  ~TestComponentCreator();
+  TestHintsComponentCreator();
+  ~TestHintsComponentCreator();
 
   // Creates component data based on |whitelisted_host_suffixes| with page hints
   // for type |optimization_type| blocking resources specified by
-  // |resource_patterns|, and returns the ComponentInfo for it.
-  optimization_guide::ComponentInfo CreateComponentInfoWithPageHints(
+  // |resource_patterns|, and returns the HintsComponentInfo for it.
+  optimization_guide::HintsComponentInfo CreateHintsComponentInfoWithPageHints(
       optimization_guide::proto::OptimizationType optimization_type,
       const std::vector<std::string>& whitelisted_host_suffixes,
       const std::vector<std::string>& resource_patterns);
 
   // Creates component data based on |whitelisted_host_suffixes| with page hints
   // for type |optimization_type| blocking resources specified by
-  // |experimental_resource_patterns|, and returns the ComponentInfo for it.
-  // The loading hints are set as experimental with experiment name set to
+  // |experimental_resource_patterns|, and returns the HintsComponentInfo for
+  // it. The loading hints are set as experimental with experiment name set to
   // kFooExperimentName.
 
   // Creates component data for testing with experimental optimizations. It
   // creates a PageHint (with page pattern "*" for each key in
   // |whitelisted_host_suffixes| that each has resource blocking patterns from
   // |experimental_resource_patterns|.
-  optimization_guide::ComponentInfo
-  CreateComponentInfoWithExperimentalPageHints(
+  optimization_guide::HintsComponentInfo
+  CreateHintsComponentInfoWithExperimentalPageHints(
       optimization_guide::proto::OptimizationType optimization_type,
       const std::vector<std::string>& whitelisted_host_suffixes,
       const std::vector<std::string>& experimental_resource_patterns);
@@ -57,7 +58,8 @@ class TestComponentCreator {
   // |whitelisted_host_suffixes| that each has resource blocking patterns from
   // |default_resource_patterns| and |experimental_resource_patterns|. The
   // experimental hints are guarded behind experiment kFooExperimentName.
-  optimization_guide::ComponentInfo CreateComponentInfoWithMixPageHints(
+  optimization_guide::HintsComponentInfo
+  CreateHintsComponentInfoWithMixPageHints(
       optimization_guide::proto::OptimizationType optimization_type,
       const std::vector<std::string>& whitelisted_host_suffixes,
       const std::vector<std::string>& experimental_resource_patterns,
@@ -75,17 +77,18 @@ class TestComponentCreator {
       const optimization_guide::proto::Configuration& config);
 
   // Writes a configuration of hints to the file path and returns the
-  // ComponentInfo for it.
-  optimization_guide::ComponentInfo WriteConfigToFileAndReturnComponentInfo(
+  // HintsComponentInfo for it.
+  optimization_guide::HintsComponentInfo
+  WriteConfigToFileAndReturnHintsComponentInfo(
       const optimization_guide::proto::Configuration& config);
 
   std::unique_ptr<base::ScopedTempDir> scoped_temp_dir_;
   int next_component_version_;
 
-  DISALLOW_COPY_AND_ASSIGN(TestComponentCreator);
+  DISALLOW_COPY_AND_ASSIGN(TestHintsComponentCreator);
 };
 
 }  // namespace testing
 }  // namespace optimization_guide
 
-#endif  // COMPONENTS_OPTIMIZATION_GUIDE_TEST_COMPONENT_CREATOR_H_
+#endif  // COMPONENTS_OPTIMIZATION_GUIDE_TEST_HINTS_COMPONENT_CREATOR_H_

components/previews/content/previews_hints.cc

@@ -15,7 +15,8 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/optional.h"
 #include "base/strings/stringprintf.h"
-#include "components/optimization_guide/optimization_guide_service_observer.h"
+#include "components/optimization_guide/hints_component_info.h"
+#include "components/optimization_guide/hints_component_util.h"
 #include "components/optimization_guide/url_pattern_with_wildcards.h"
 #include "components/previews/core/bloom_filter.h"
 #include "components/previews/core/previews_features.h"
@@ -39,7 +40,7 @@ const base::FilePath::CharType kSentinelFileName[] =
 // Returns false if the processing should not continue because the
 // file exists with the same version (indicating that processing that version
 // failed previously (possibly crash or shutdown). Should be run in the
-// background (e.g., same task as Hints.CreateFromConfig).
+// background (e.g., same task as PreviewsHints::CreateFromHintsComponent()).
 bool CreateSentinelFile(const base::FilePath& sentinel_path,
                         const base::Version& version) {
   DCHECK(version.IsValid());
@@ -297,12 +298,16 @@ PreviewsHints::~PreviewsHints() {
 }
 
 // static
-std::unique_ptr<PreviewsHints> PreviewsHints::CreateFromConfig(
-    const optimization_guide::proto::Configuration& config,
-    const optimization_guide::ComponentInfo& info) {
-  base::FilePath sentinel_path(
-      info.hints_path.DirName().Append(kSentinelFileName));
-  if (!CreateSentinelFile(sentinel_path, info.hints_version)) {
+std::unique_ptr<PreviewsHints> PreviewsHints::CreateFromHintsComponent(
+    const optimization_guide::HintsComponentInfo& info) {
+  std::unique_ptr<optimization_guide::proto::Configuration> config =
+      ProcessHintsComponent(info);
+  if (!config) {
+    return nullptr;
+  }
+
+  base::FilePath sentinel_path(info.path.DirName().Append(kSentinelFileName));
+  if (!CreateSentinelFile(sentinel_path, info.version)) {
     std::unique_ptr<PreviewsHints> no_hints;
     RecordProcessHintsResult(
         PreviewsProcessHintsResult::kFailedFinishProcessing);
@@ -323,7 +328,7 @@ std::unique_ptr<PreviewsHints> PreviewsHints::CreateFromConfig(
   size_t total_page_patterns_with_resource_loading_hints_received = 0;
   size_t total_resource_loading_hints_received = 0;
   // Process hint configuration.
-  for (const auto& hint : config.hints()) {
+  for (const auto& hint : config->hints()) {
     // We only support host suffixes at the moment. Skip anything else.
     // One |hint| applies to one host URL suffix.
     if (hint.key_representation() != optimization_guide::proto::HOST_SUFFIX)
@@ -400,7 +405,7 @@ std::unique_ptr<PreviewsHints> PreviewsHints::CreateFromConfig(
   }
 
   // Extract any supported large scale blacklists from the configuration.
-  hints->ParseOptimizationFilters(config);
+  hints->ParseOptimizationFilters(*config);
 
   // Completed processing hints data without crashing so clear sentinel.
   DeleteSentinelFile(sentinel_path);

components/previews/content/previews_hints.h

@@ -24,7 +24,7 @@
 class GURL;
 
 namespace optimization_guide {
-struct ComponentInfo;
+struct HintsComponentInfo;
 }
 
 namespace previews {
@@ -34,10 +34,11 @@ class PreviewsHints {
  public:
   ~PreviewsHints();
 
-  // Creates a Hints instance from the provided configuration.
-  static std::unique_ptr<PreviewsHints> CreateFromConfig(
-      const optimization_guide::proto::Configuration& config,
-      const optimization_guide::ComponentInfo& info);
+  // Creates a Hints instance from the provided hints component. This must be
+  // called using a background task runner as it requires a significant amount
+  // of processing.
+  static std::unique_ptr<PreviewsHints> CreateFromHintsComponent(
+      const optimization_guide::HintsComponentInfo& info);
 
   static std::unique_ptr<PreviewsHints> CreateForTesting(
       std::unique_ptr<HostFilter> lite_page_redirect_blacklist);

components/previews/content/previews_hints_unittest.cc

@@ -4,11 +4,14 @@
 
 #include "components/previews/content/previews_hints.h"
 
+#include <string>
+
 #include "base/command_line.h"
+#include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
-#include "components/optimization_guide/optimization_guide_service_observer.h"
+#include "components/optimization_guide/hints_component_info.h"
 #include "components/optimization_guide/proto/hints.pb.h"
 #include "components/previews/core/previews_features.h"
 #include "components/previews/core/previews_switches.h"
@@ -32,17 +35,18 @@ class TestHostFilter : public previews::HostFilter {
 
 class PreviewsHintsTest : public testing::Test {
  public:
-  explicit PreviewsHintsTest() : previews_hints_(nullptr) {}
+  PreviewsHintsTest() : previews_hints_(nullptr) {}
 
   ~PreviewsHintsTest() override {}
 
   void SetUp() override { ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); }
 
   void ParseConfig(const optimization_guide::proto::Configuration& config) {
-    optimization_guide::ComponentInfo info(
+    optimization_guide::HintsComponentInfo info(
         base::Version("1.0"),
         temp_dir_.GetPath().Append(FILE_PATH_LITERAL("somefile.pb")));
-    previews_hints_ = PreviewsHints::CreateFromConfig(config, info);
+    ASSERT_NO_FATAL_FAILURE(WriteConfigToFile(config, info.path));
+    previews_hints_ = PreviewsHints::CreateFromHintsComponent(info);
     previews_hints_->Initialize();
   }
 
@@ -53,6 +57,15 @@ class PreviewsHintsTest : public testing::Test {
   }
 
  private:
+  void WriteConfigToFile(const optimization_guide::proto::Configuration& config,
+                         const base::FilePath& filePath) {
+    std::string serialized_config;
+    ASSERT_TRUE(config.SerializeToString(&serialized_config));
+    ASSERT_EQ(static_cast<int32_t>(serialized_config.length()),
+              base::WriteFile(filePath, serialized_config.data(),
+                              serialized_config.length()));
+  }
+
   base::ScopedTempDir temp_dir_;
   std::unique_ptr<PreviewsHints> previews_hints_;
 };

components/previews/content/previews_optimization_guide.cc

@@ -8,9 +8,12 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/task/post_task.h"
 #include "base/task_runner_util.h"
+#include "components/optimization_guide/hints_component_info.h"
+#include "components/optimization_guide/optimization_guide_service.h"
 #include "components/optimization_guide/proto/hints.pb.h"
 #include "components/previews/content/previews_hints.h"
 #include "components/previews/content/previews_user_data.h"
+#include "components/previews/core/previews_constants.h"
 #include "url/gurl.h"
 
 namespace previews {
@@ -122,14 +125,13 @@ void PreviewsOptimizationGuide::LogHintCacheMatch(
   hints_->LogHintCacheMatch(url, is_committed, ect);
 }
 
-void PreviewsOptimizationGuide::OnHintsProcessed(
-    const optimization_guide::proto::Configuration& config,
-    const optimization_guide::ComponentInfo& info) {
+void PreviewsOptimizationGuide::OnHintsComponentAvailable(
+    const optimization_guide::HintsComponentInfo& info) {
   DCHECK(ui_task_runner_->BelongsToCurrentThread());
 
   base::PostTaskAndReplyWithResult(
       background_task_runner_.get(), FROM_HERE,
-      base::BindOnce(&PreviewsHints::CreateFromConfig, config, info),
+      base::BindOnce(&PreviewsHints::CreateFromHintsComponent, info),
       base::BindOnce(&PreviewsOptimizationGuide::UpdateHints,
                      ui_weak_ptr_factory_.GetWeakPtr()));
 }
@@ -138,8 +140,15 @@ void PreviewsOptimizationGuide::UpdateHints(
     std::unique_ptr<PreviewsHints> hints) {
   DCHECK(ui_task_runner_->BelongsToCurrentThread());
   hints_ = std::move(hints);
-  if (hints_)
+  if (hints_) {
     hints_->Initialize();
+  }
+
+  // Record the result of updating the hints. This is used as a signal for the
+  // hints being fully processed in testing.
+  LOCAL_HISTOGRAM_BOOLEAN(
+      kPreviewsOptimizationGuideUpdateHintsResultHistogramString,
+      hints_ != NULL);
 }
 
 }  // namespace previews

components/previews/content/previews_optimization_guide.h

@@ -13,15 +13,16 @@
 #include "base/memory/weak_ptr.h"
 #include "base/sequenced_task_runner.h"
 #include "base/single_thread_task_runner.h"
-#include "components/optimization_guide/optimization_guide_service.h"
 #include "components/optimization_guide/optimization_guide_service_observer.h"
 #include "components/previews/content/previews_optimization_guide.h"
 #include "components/previews/core/previews_experiments.h"
 #include "url/gurl.h"
 
 namespace optimization_guide {
+struct HintsComponentInfo;
+class OptimizationGuideService;
 namespace proto {
-class Configuration;
+class Hint;
 }  // namespace proto
 }  // namespace optimization_guide
 
@@ -75,9 +76,8 @@ class PreviewsOptimizationGuide
                          net::EffectiveConnectionType ect) const;
 
   // optimization_guide::OptimizationGuideServiceObserver implementation:
-  void OnHintsProcessed(
-      const optimization_guide::proto::Configuration& config,
-      const optimization_guide::ComponentInfo& component_info) override;
+  void OnHintsComponentAvailable(
+      const optimization_guide::HintsComponentInfo& info) override;
 
  private:
   // Updates the hints to the latest hints sent by the Component Updater.

components/previews/content/previews_optimization_guide_unittest.cc

@@ -19,8 +19,8 @@
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/test/scoped_task_environment.h"
+#include "components/optimization_guide/hints_component_info.h"
 #include "components/optimization_guide/optimization_guide_service.h"
-#include "components/optimization_guide/optimization_guide_service_observer.h"
 #include "components/optimization_guide/proto/hints.pb.h"
 #include "components/previews/content/previews_user_data.h"
 #include "components/previews/core/bloom_filter.h"
@@ -80,11 +80,12 @@ class PreviewsOptimizationGuideTest : public testing::Test {
   }
 
   void ProcessHints(const optimization_guide::proto::Configuration& config,
-                    std::string version) {
-    optimization_guide::ComponentInfo info(
+                    const std::string& version) {
+    optimization_guide::HintsComponentInfo info(
         base::Version(version),
         temp_dir().Append(FILE_PATH_LITERAL("somefile.pb")));
-    guide_->OnHintsProcessed(config, info);
+    ASSERT_NO_FATAL_FAILURE(WriteConfigToFile(config, info.path));
+    guide_->OnHintsComponentAvailable(info);
   }
 
   void MaybeLoadOptimizationHintsCallback(
@@ -137,6 +138,15 @@ class PreviewsOptimizationGuideTest : public testing::Test {
   void InitializeWithLitePageRedirectBlacklist();
 
  private:
+  void WriteConfigToFile(const optimization_guide::proto::Configuration& config,
+                         const base::FilePath& filePath) {
+    std::string serialized_config;
+    ASSERT_TRUE(config.SerializeToString(&serialized_config));
+    ASSERT_EQ(static_cast<int32_t>(serialized_config.length()),
+              base::WriteFile(filePath, serialized_config.data(),
+                              serialized_config.length()));
+  }
+
   base::test::ScopedTaskEnvironment scoped_task_environment_;
   base::ScopedTempDir temp_dir_;
 

components/previews/core/BUILD.gn

@@ -10,6 +10,8 @@ static_library("core") {
     "host_filter.h",
     "previews_black_list.cc",
     "previews_black_list.h",
+    "previews_constants.cc",
+    "previews_constants.h",
     "previews_decider.h",
     "previews_experiments.cc",
     "previews_experiments.h",

components/previews/core/previews_constants.cc

+// Copyright 2018 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/previews/core/previews_constants.h"
+
+namespace previews {
+
+const char kPreviewsOptimizationGuideUpdateHintsResultHistogramString[] =
+    "PreviewsOptimizationGuide.UpdateHints.Result";
+
+}  // namespace previews

components/previews/core/previews_constants.h

+// Copyright 2018 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_PREVIEWS_CORE_PREVIEWS_CONSTANTS_H_
+#define COMPONENTS_PREVIEWS_CORE_PREVIEWS_CONSTANTS_H_
+
+namespace previews {
+
+// The local histogram used by PreviewsOptimizationGuide to record the result of
+// UpdateHints().
+extern const char kPreviewsOptimizationGuideUpdateHintsResultHistogramString[];
+
+}  // namespace previews
+
+#endif  // COMPONENTS_PREVIEWS_CORE_PREVIEWS_CONSTANTS_H_