Revert "Eliminate unnecessary proto::Configuration copies"

This reverts commit 1b420fdb.

Reason for revert: Makes PreviewsNoScriptBrowserTest and 
ResourceLoadingHintsBrowserTest flaky

https://crbug.com/909998
https://crbug.com/909999


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
> 
> Change-Id: I330fcc35f14388b8a704d3180bd758e7327e9892
> Reviewed-on: https://chromium-review.googlesource.com/c/1351719
> 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@{#611982}

TBR=waffles@chromium.org,tbansal@chromium.org,dougarnett@chromium.org,ryansturm@chromium.org,jegray@chromium.org

Change-Id: I65e2ffff8737cc89bcc0726b4c3e67271e654c2d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 908985
Reviewed-on: https://chromium-review.googlesource.com/c/1354745Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612044}

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::HintsComponentInfo info(
+    optimization_guide::ComponentInfo component_info(
         version,
         install_dir.Append(optimization_guide::kUnindexedHintsFileName));
-    optimization_guide_service->MaybeUpdateHintsComponent(info);
+    optimization_guide_service->ProcessHints(component_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 MaybeUpdateHintsComponent(
-      const optimization_guide::HintsComponentInfo& info) override {
-    hints_component_info_ =
-        std::make_unique<optimization_guide::HintsComponentInfo>(info);
+  void ProcessHints(
+      const optimization_guide::ComponentInfo& component_info) override {
+    component_info_ =
+        std::make_unique<optimization_guide::ComponentInfo>(component_info);
   }
 
-  optimization_guide::HintsComponentInfo* hints_component_info() const {
-    return hints_component_info_.get();
+  optimization_guide::ComponentInfo* component_info() const {
+    return component_info_.get();
   }
 
  private:
-  std::unique_ptr<optimization_guide::HintsComponentInfo> hints_component_info_;
+  std::unique_ptr<optimization_guide::ComponentInfo> 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()->hints_component_info());
+  EXPECT_EQ(nullptr, service()->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()->hints_component_info());
+  EXPECT_EQ(nullptr, service()->component_info());
 }
 
 TEST_F(OptimizationHintsComponentInstallerTest, LoadFileWithData) {
@@ -227,11 +227,12 @@ TEST_F(OptimizationHintsComponentInstallerTest, LoadFileWithData) {
   ASSERT_NO_FATAL_FAILURE(CreateTestOptimizationHints(expected_hints));
   ASSERT_NO_FATAL_FAILURE(LoadOptimizationHints(ruleset_format_version()));
 
-  auto* component_info = service()->hints_component_info();
+  auto* component_info = service()->component_info();
   EXPECT_NE(nullptr, component_info);
-  EXPECT_EQ(base::Version(kTestHintsVersion), component_info->version);
+  EXPECT_EQ(base::Version(kTestHintsVersion), component_info->hints_version);
   std::string actual_hints;
-  ASSERT_TRUE(base::ReadFileToString(component_info->path, &actual_hints));
+  ASSERT_TRUE(
+      base::ReadFileToString(component_info->hints_path, &actual_hints));
   EXPECT_EQ(expected_hints, actual_hints);
 }
 

chrome/browser/previews/previews_browsertest.cc

@@ -11,17 +11,15 @@
 #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_hints_component_creator.h"
-#include "components/previews/core/previews_constants.h"
+#include "components/optimization_guide/test_component_creator.h"
 #include "components/previews/core/previews_features.h"
 #include "components/previews/core/previews_switches.h"
 #include "content/public/browser/browser_task_traits.h"
@@ -32,27 +30,32 @@
 
 namespace {
 
-// 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) {
-    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)
-      return;
-    content::FetchHistogramsFromChildProcesses();
-    SubprocessMetricsProvider::MergeHistogramDeltasForTesting();
-    base::RunLoop().RunUntilIdle();
+// 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();
   }
-  // If this is reached, then automatically fail the test.
-  FAIL() << histogram_name << " did not reach expected sample count of "
-         << count << ".";
-}
+
+  std::unique_ptr<base::RunLoop> run_loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestOptimizationGuideServiceObserver);
+};
 
 }  // namespace
 
@@ -210,26 +213,25 @@ class PreviewsNoScriptBrowserTest : public PreviewsBrowserTest {
 
   void SetUpNoScriptWhitelist(
       std::vector<std::string> whitelisted_noscript_sites) {
-    const optimization_guide::HintsComponentInfo& component_info =
-        test_hints_component_creator_.CreateHintsComponentInfoWithPageHints(
+    TestOptimizationGuideServiceObserver observer;
+    g_browser_process->optimization_guide_service()->AddObserver(&observer);
+    base::RunLoop().RunUntilIdle();
+
+    const optimization_guide::ComponentInfo& component_info =
+        test_component_creator_.CreateComponentInfoWithPageHints(
             optimization_guide::proto::NOSCRIPT, whitelisted_noscript_sites,
             {});
-
-    base::HistogramTester histogram_tester;
-
-    g_browser_process->optimization_guide_service()->MaybeUpdateHintsComponent(
+    g_browser_process->optimization_guide_service()->ProcessHints(
         component_info);
 
-    RetryForHistogramUntilCountReached(
-        &histogram_tester,
-        previews::kPreviewsOptimizationGuideUpdateHintsResultHistogramString,
-        1);
+    // Wait for hints to be processed by PreviewsOptimizationGuide.
+    observer.WaitForNotification();
+    base::RunLoop().RunUntilIdle();
   }
 
  private:
   base::test::ScopedFeatureList scoped_feature_list_;
-  optimization_guide::testing::TestHintsComponentCreator
-      test_hints_component_creator_;
+  optimization_guide::testing::TestComponentCreator test_component_creator_;
 };
 
 // Previews InfoBar (which these tests triggers) does not work on Mac.

components/optimization_guide/BUILD.gn

@@ -4,9 +4,6 @@
 
 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",
@@ -25,8 +22,8 @@ static_library("optimization_guide") {
 static_library("test_support") {
   testonly = true
   sources = [
-    "test_hints_component_creator.cc",
-    "test_hints_component_creator.h",
+    "test_component_creator.cc",
+    "test_component_creator.h",
   ]
   deps = [
     ":optimization_guide",
@@ -39,7 +36,6 @@ 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,26 +4,59 @@
 
 #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)
-    : ui_thread_task_runner_(ui_thread_task_runner), weak_ptr_factory_(this) {
+    : background_task_runner_(base::CreateSequencedTaskRunnerWithTraits(
+          {base::MayBlock(), base::TaskPriority::BEST_EFFORT})),
+      ui_thread_task_runner_(ui_thread_task_runner),
+      latest_processed_version_(kNullVersion) {
   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(
@@ -32,33 +65,55 @@ void OptimizationGuideService::RemoveObserver(
   observers_.RemoveObserver(observer);
 }
 
-void OptimizationGuideService::MaybeUpdateHintsComponent(
-    const HintsComponentInfo& info) {
-  ui_thread_task_runner_->PostTask(
+void OptimizationGuideService::ProcessHints(
+    const ComponentInfo& component_info) {
+  background_task_runner_->PostTask(
       FROM_HERE,
-      base::BindOnce(
-          &OptimizationGuideService::MaybeUpdateHintsComponentOnUIThread,
-          weak_ptr_factory_.GetWeakPtr(), info));
+      base::BindOnce(&OptimizationGuideService::ProcessHintsInBackground,
+                     base::Unretained(this), component_info));
 }
 
-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) {
+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);
     return;
   }
 
-  hints_component_info_.emplace(info.version, info.path);
-  for (auto& observer : observers_) {
-    observer.OnHintsComponentAvailable(*hints_component_info_);
+  proto::Configuration new_config;
+  if (!new_config.ParseFromString(binary_pb)) {
+    RecordProcessHintsResult(ProcessHintsResult::FAILED_INVALID_CONFIGURATION);
+    return;
   }
+  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,47 +5,69 @@
 #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/weak_ptr.h"
+#include "base/memory/ref_counted.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 "components/optimization_guide/hints_component_info.h"
+#include "base/version.h"
 #include "components/optimization_guide/optimization_guide_service_observer.h"
+#include "components/optimization_guide/proto/hints.pb.h"
 
 namespace optimization_guide {
 
-// Tracks the info for the current Optimization Hints component and notifies
-// observers of newly available hints components downloaded from the Component
-// Updater.
+// Processes the hints downloaded from the Component Updater as part of the
+// Optimization Hints component.
 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);
 
-  // Forwards the update hints component request on to the UI thread, where the
-  // actual work occurs.
+  // 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.
   //
   // Virtual so it can be mocked out in tests.
-  virtual void MaybeUpdateHintsComponent(const HintsComponentInfo& info);
+  virtual void ProcessHints(const ComponentInfo& component_info);
+
+  // Sets the latest processed version for testing.
+  void SetLatestProcessedVersionForTesting(const base::Version& version);
 
  private:
-  // 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);
+  // 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);
 
   // Runner for indexing tasks.
+  scoped_refptr<base::SequencedTaskRunner> background_task_runner_;
   SEQUENCE_CHECKER(sequence_checker_);
 
   // Runner for UI Thread tasks.
@@ -54,12 +76,7 @@ class OptimizationGuideService {
   // Observers receiving notifications on hints being processed.
   base::ObserverList<OptimizationGuideServiceObserver>::Unchecked observers_;
 
-  // 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_;
+  base::Version latest_processed_version_;
 
   DISALLOW_COPY_AND_ASSIGN(OptimizationGuideService);
 };

components/optimization_guide/optimization_guide_service_observer.h

@@ -5,20 +5,35 @@
 #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 {
 
-struct HintsComponentInfo;
+// 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;
+};
 
 // Interface for objects that wish to be notified of changes in the Optimization
 // Guide Service.
 //
-// All calls will be made on the UI thread.
+// All calls will be made on the IO thread.
 class OptimizationGuideServiceObserver {
  public:
-  // 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;
+  // Called when the hints have been processed.
+  virtual void OnHintsProcessed(
+      const proto::Configuration& config,
+      const optimization_guide::ComponentInfo& component_info) = 0;
 
  protected:
   virtual ~OptimizationGuideServiceObserver() {}

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

@@ -2,28 +2,29 @@
 // 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_hints_component_creator.h"
+#include "components/optimization_guide/test_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 {
 
-TestHintsComponentCreator::TestHintsComponentCreator()
+TestComponentCreator::TestComponentCreator()
     : scoped_temp_dir_(std::make_unique<base::ScopedTempDir>()),
       next_component_version_(1) {}
 
-TestHintsComponentCreator::~TestHintsComponentCreator() {
+TestComponentCreator::~TestComponentCreator() {
   base::ScopedAllowBlockingForTesting allow_blocking;
   scoped_temp_dir_.reset();
 }
 
-optimization_guide::HintsComponentInfo
-TestHintsComponentCreator::CreateHintsComponentInfoWithPageHints(
+optimization_guide::ComponentInfo
+TestComponentCreator::CreateComponentInfoWithPageHints(
     optimization_guide::proto::OptimizationType optimization_type,
     const std::vector<std::string>& page_hint_host_suffixes,
     const std::vector<std::string>& resource_blocking_patterns) {
@@ -49,11 +50,11 @@ TestHintsComponentCreator::CreateHintsComponentInfoWithPageHints(
     }
   }
 
-  return WriteConfigToFileAndReturnHintsComponentInfo(config);
+  return WriteConfigToFileAndReturnComponentInfo(config);
 }
 
-optimization_guide::HintsComponentInfo
-TestHintsComponentCreator::CreateHintsComponentInfoWithExperimentalPageHints(
+optimization_guide::ComponentInfo
+TestComponentCreator::CreateComponentInfoWithExperimentalPageHints(
     optimization_guide::proto::OptimizationType optimization_type,
     const std::vector<std::string>& page_hint_host_suffixes,
     const std::vector<std::string>& experimental_resource_patterns) {
@@ -80,11 +81,11 @@ TestHintsComponentCreator::CreateHintsComponentInfoWithExperimentalPageHints(
     }
   }
 
-  return WriteConfigToFileAndReturnHintsComponentInfo(config);
+  return WriteConfigToFileAndReturnComponentInfo(config);
 }
 
-optimization_guide::HintsComponentInfo
-TestHintsComponentCreator::CreateHintsComponentInfoWithMixPageHints(
+optimization_guide::ComponentInfo
+TestComponentCreator::CreateComponentInfoWithMixPageHints(
     optimization_guide::proto::OptimizationType optimization_type,
     const std::vector<std::string>& page_hint_host_suffixes,
     const std::vector<std::string>& experimental_resource_patterns,
@@ -129,18 +130,17 @@ TestHintsComponentCreator::CreateHintsComponentInfoWithMixPageHints(
     }
   }
 
-  return WriteConfigToFileAndReturnHintsComponentInfo(config);
+  return WriteConfigToFileAndReturnComponentInfo(config);
 }
 
-base::FilePath TestHintsComponentCreator::GetFilePath(
-    std::string file_path_suffix) {
+base::FilePath TestComponentCreator::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 TestHintsComponentCreator::WriteConfigToFile(
+void TestComponentCreator::WriteConfigToFile(
     const base::FilePath& file_path,
     const optimization_guide::proto::Configuration& config) {
   base::ScopedAllowBlockingForTesting allow_blocking;
@@ -153,14 +153,14 @@ void TestHintsComponentCreator::WriteConfigToFile(
                             serialized_config.length()));
 }
 
-optimization_guide::HintsComponentInfo
-TestHintsComponentCreator::WriteConfigToFileAndReturnHintsComponentInfo(
+optimization_guide::ComponentInfo
+TestComponentCreator::WriteConfigToFileAndReturnComponentInfo(
     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::HintsComponentInfo(base::Version(version_string),
-                                                file_path);
+  return optimization_guide::ComponentInfo(base::Version(version_string),
+                                           file_path);
 }
 
 }  // namespace testing

components/optimization_guide/test_hints_component_creator.h → components/optimization_guide/test_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_HINTS_COMPONENT_CREATOR_H_
-#define COMPONENTS_OPTIMIZATION_GUIDE_TEST_HINTS_COMPONENT_CREATOR_H_
+#ifndef COMPONENTS_OPTIMIZATION_GUIDE_TEST_COMPONENT_CREATOR_H_
+#define COMPONENTS_OPTIMIZATION_GUIDE_TEST_COMPONENT_CREATOR_H_
 
 #include <string>
 #include <vector>
@@ -12,7 +12,6 @@
 #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 {
@@ -24,31 +23,31 @@ static const char kFooExperimentName[] = "foo_experiment";
 //
 // All temporary files and paths are cleaned up when this instance goes out of
 // scope.
-class TestHintsComponentCreator {
+class TestComponentCreator {
  public:
-  TestHintsComponentCreator();
-  ~TestHintsComponentCreator();
+  TestComponentCreator();
+  ~TestComponentCreator();
 
   // Creates component data based on |whitelisted_host_suffixes| with page hints
   // for type |optimization_type| blocking resources specified by
-  // |resource_patterns|, and returns the HintsComponentInfo for it.
-  optimization_guide::HintsComponentInfo CreateHintsComponentInfoWithPageHints(
+  // |resource_patterns|, and returns the ComponentInfo for it.
+  optimization_guide::ComponentInfo CreateComponentInfoWithPageHints(
       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 HintsComponentInfo for
-  // it. The loading hints are set as experimental with experiment name set to
+  // |experimental_resource_patterns|, and returns the ComponentInfo 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::HintsComponentInfo
-  CreateHintsComponentInfoWithExperimentalPageHints(
+  optimization_guide::ComponentInfo
+  CreateComponentInfoWithExperimentalPageHints(
       optimization_guide::proto::OptimizationType optimization_type,
       const std::vector<std::string>& whitelisted_host_suffixes,
       const std::vector<std::string>& experimental_resource_patterns);
@@ -58,8 +57,7 @@ class TestHintsComponentCreator {
   // |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::HintsComponentInfo
-  CreateHintsComponentInfoWithMixPageHints(
+  optimization_guide::ComponentInfo CreateComponentInfoWithMixPageHints(
       optimization_guide::proto::OptimizationType optimization_type,
       const std::vector<std::string>& whitelisted_host_suffixes,
       const std::vector<std::string>& experimental_resource_patterns,
@@ -77,18 +75,17 @@ class TestHintsComponentCreator {
       const optimization_guide::proto::Configuration& config);
 
   // Writes a configuration of hints to the file path and returns the
-  // HintsComponentInfo for it.
-  optimization_guide::HintsComponentInfo
-  WriteConfigToFileAndReturnHintsComponentInfo(
+  // ComponentInfo for it.
+  optimization_guide::ComponentInfo WriteConfigToFileAndReturnComponentInfo(
       const optimization_guide::proto::Configuration& config);
 
   std::unique_ptr<base::ScopedTempDir> scoped_temp_dir_;
   int next_component_version_;
 
-  DISALLOW_COPY_AND_ASSIGN(TestHintsComponentCreator);
+  DISALLOW_COPY_AND_ASSIGN(TestComponentCreator);
 };
 
 }  // namespace testing
 }  // namespace optimization_guide
 
-#endif  // COMPONENTS_OPTIMIZATION_GUIDE_TEST_HINTS_COMPONENT_CREATOR_H_
+#endif  // COMPONENTS_OPTIMIZATION_GUIDE_TEST_COMPONENT_CREATOR_H_

components/previews/content/previews_hints.cc

@@ -15,8 +15,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/optional.h"
 #include "base/strings/stringprintf.h"
-#include "components/optimization_guide/hints_component_info.h"
-#include "components/optimization_guide/hints_component_util.h"
+#include "components/optimization_guide/optimization_guide_service_observer.h"
 #include "components/optimization_guide/url_pattern_with_wildcards.h"
 #include "components/previews/core/bloom_filter.h"
 #include "components/previews/core/previews_features.h"
@@ -40,7 +39,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 PreviewsHints::CreateFromHintsComponent()).
+// background (e.g., same task as Hints.CreateFromConfig).
 bool CreateSentinelFile(const base::FilePath& sentinel_path,
                         const base::Version& version) {
   DCHECK(version.IsValid());
@@ -298,16 +297,12 @@ PreviewsHints::~PreviewsHints() {
 }
 
 // static
-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> 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> no_hints;
     RecordProcessHintsResult(
         PreviewsProcessHintsResult::kFailedFinishProcessing);
@@ -328,7 +323,7 @@ std::unique_ptr<PreviewsHints> PreviewsHints::CreateFromHintsComponent(
   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)
@@ -405,7 +400,7 @@ std::unique_ptr<PreviewsHints> PreviewsHints::CreateFromHintsComponent(
   }
 
   // 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 HintsComponentInfo;
+struct ComponentInfo;
 }
 
 namespace previews {
@@ -34,11 +34,10 @@ class PreviewsHints {
  public:
   ~PreviewsHints();
 
-  // 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);
+  // 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);
 
   static std::unique_ptr<PreviewsHints> CreateForTesting(
       std::unique_ptr<HostFilter> lite_page_redirect_blacklist);

components/previews/content/previews_hints_unittest.cc

@@ -4,14 +4,11 @@
 
 #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/hints_component_info.h"
+#include "components/optimization_guide/optimization_guide_service_observer.h"
 #include "components/optimization_guide/proto/hints.pb.h"
 #include "components/previews/core/previews_features.h"
 #include "components/previews/core/previews_switches.h"
@@ -35,18 +32,17 @@ class TestHostFilter : public previews::HostFilter {
 
 class PreviewsHintsTest : public testing::Test {
  public:
-  PreviewsHintsTest() : previews_hints_(nullptr) {}
+  explicit PreviewsHintsTest() : previews_hints_(nullptr) {}
 
   ~PreviewsHintsTest() override {}
 
   void SetUp() override { ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); }
 
   void ParseConfig(const optimization_guide::proto::Configuration& config) {
-    optimization_guide::HintsComponentInfo info(
+    optimization_guide::ComponentInfo info(
         base::Version("1.0"),
         temp_dir_.GetPath().Append(FILE_PATH_LITERAL("somefile.pb")));
-    ASSERT_NO_FATAL_FAILURE(WriteConfigToFile(config, info.path));
-    previews_hints_ = PreviewsHints::CreateFromHintsComponent(info);
+    previews_hints_ = PreviewsHints::CreateFromConfig(config, info);
     previews_hints_->Initialize();
   }
 
@@ -57,15 +53,6 @@ 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,12 +8,9 @@
 #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 {
@@ -125,13 +122,14 @@ void PreviewsOptimizationGuide::LogHintCacheMatch(
   hints_->LogHintCacheMatch(url, is_committed, ect);
 }
 
-void PreviewsOptimizationGuide::OnHintsComponentAvailable(
-    const optimization_guide::HintsComponentInfo& info) {
+void PreviewsOptimizationGuide::OnHintsProcessed(
+    const optimization_guide::proto::Configuration& config,
+    const optimization_guide::ComponentInfo& info) {
   DCHECK(ui_task_runner_->BelongsToCurrentThread());
 
   base::PostTaskAndReplyWithResult(
       background_task_runner_.get(), FROM_HERE,
-      base::BindOnce(&PreviewsHints::CreateFromHintsComponent, info),
+      base::BindOnce(&PreviewsHints::CreateFromConfig, config, info),
       base::BindOnce(&PreviewsOptimizationGuide::UpdateHints,
                      ui_weak_ptr_factory_.GetWeakPtr()));
 }
@@ -140,15 +138,8 @@ 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,16 +13,15 @@
 #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 Hint;
+class Configuration;
 }  // namespace proto
 }  // namespace optimization_guide
 
@@ -76,8 +75,9 @@ class PreviewsOptimizationGuide
                          net::EffectiveConnectionType ect) const;
 
   // optimization_guide::OptimizationGuideServiceObserver implementation:
-  void OnHintsComponentAvailable(
-      const optimization_guide::HintsComponentInfo& info) override;
+  void OnHintsProcessed(
+      const optimization_guide::proto::Configuration& config,
+      const optimization_guide::ComponentInfo& component_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,12 +80,11 @@ class PreviewsOptimizationGuideTest : public testing::Test {
   }
 
   void ProcessHints(const optimization_guide::proto::Configuration& config,
-                    const std::string& version) {
-    optimization_guide::HintsComponentInfo info(
+                    std::string version) {
+    optimization_guide::ComponentInfo info(
         base::Version(version),
         temp_dir().Append(FILE_PATH_LITERAL("somefile.pb")));
-    ASSERT_NO_FATAL_FAILURE(WriteConfigToFile(config, info.path));
-    guide_->OnHintsComponentAvailable(info);
+    guide_->OnHintsProcessed(config, info);
   }
 
   void MaybeLoadOptimizationHintsCallback(
@@ -138,15 +137,6 @@ 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,8 +10,6 @@ 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_