Fix flaky test in navigation predictor

The test is still flaky, although less than before.

This CL replaces the default constructor by an explicit one which should
copy the set of URLs and help with the flakiness.

Change-Id: I2205d681a62fb19de5cabe675504503facdf7d2b
Bug: 1065285
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2136886
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Auto-Submit: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Robert Ogden <robertogden@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757200}

chrome/browser/navigation_predictor/navigation_predictor_browsertest.cc

@@ -4,6 +4,7 @@
 
 #include "base/macros.h"
 #include "base/run_loop.h"
+#include "base/sequence_checker.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
@@ -131,18 +132,25 @@ class NavigationPredictorBrowserTest
 class TestObserver : public NavigationPredictorKeyedService::Observer {
  public:
   TestObserver() {}
-  ~TestObserver() override {}
+  ~TestObserver() override {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  }
 
   base::Optional<NavigationPredictorKeyedService::Prediction> last_prediction()
       const {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
     return last_prediction_;
   }
 
-  size_t count_predictions() const { return count_predictions_; }
+  size_t count_predictions() const {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    return count_predictions_;
+  }
 
   // Waits until the count if received notifications is at least
   // |expected_notifications_count|.
   void WaitUntilNotificationsCountReached(size_t expected_notifications_count) {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
     // Ensure that |wait_loop_| is null implying there is no ongoing wait.
     ASSERT_FALSE(!!wait_loop_);
 
@@ -158,6 +166,7 @@ class TestObserver : public NavigationPredictorKeyedService::Observer {
   void OnPredictionUpdated(
       const base::Optional<NavigationPredictorKeyedService::Prediction>
           prediction) override {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
     ++count_predictions_;
     last_prediction_ = prediction;
     if (wait_loop_ && count_predictions_ >= expected_notifications_count_) {
@@ -176,6 +185,8 @@ class TestObserver : public NavigationPredictorKeyedService::Observer {
   std::unique_ptr<base::RunLoop> wait_loop_;
   base::Optional<size_t> expected_notifications_count_;
 
+  SEQUENCE_CHECKER(sequence_checker_);
+
   DISALLOW_COPY_AND_ASSIGN(TestObserver);
 };
 

chrome/browser/navigation_predictor/navigation_predictor_keyed_service.cc

@@ -37,11 +37,57 @@ NavigationPredictorKeyedService::Prediction::Prediction(
 }
 
 NavigationPredictorKeyedService::Prediction::Prediction(
-    const NavigationPredictorKeyedService::Prediction& other) = default;
+    const NavigationPredictorKeyedService::Prediction& other)
+    : web_contents_(other.web_contents_),
+      prediction_source_(other.prediction_source_) {
+  // Use non-default copy constructor operator that does deep-copy.
+  if (other.source_document_url_)
+    source_document_url_ = other.source_document_url_;
+
+  if (other.external_app_packages_name_) {
+    external_app_packages_name_ = std::vector<std::string>();
+    external_app_packages_name_->reserve(
+        other.external_app_packages_name_->size());
+    for (const auto& entry : other.external_app_packages_name_.value())
+      external_app_packages_name_->push_back(entry);
+  }
+
+  sorted_predicted_urls_.reserve(other.sorted_predicted_urls_.size());
+  for (const auto& entry : other.sorted_predicted_urls_)
+    sorted_predicted_urls_.push_back(entry);
+}
 
 NavigationPredictorKeyedService::Prediction&
 NavigationPredictorKeyedService::Prediction::operator=(
-    const NavigationPredictorKeyedService::Prediction& other) = default;
+    const NavigationPredictorKeyedService::Prediction& other) {
+  // Use non-default assignment operator that does deep-copy.
+  web_contents_ = other.web_contents_;
+
+  source_document_url_.reset();
+  if (other.source_document_url_)
+    source_document_url_ = other.source_document_url_;
+
+  if (external_app_packages_name_)
+    external_app_packages_name_.reset();
+
+  if (other.external_app_packages_name_) {
+    external_app_packages_name_ = std::vector<std::string>();
+    external_app_packages_name_->reserve(
+        other.external_app_packages_name_->size());
+    for (const auto& entry : other.external_app_packages_name_.value())
+      external_app_packages_name_->push_back(entry);
+  } else {
+    external_app_packages_name_.reset();
+  }
+  prediction_source_ = other.prediction_source_;
+
+  sorted_predicted_urls_.clear();
+  sorted_predicted_urls_.reserve(other.sorted_predicted_urls_.size());
+  for (const auto& entry : other.sorted_predicted_urls_)
+    sorted_predicted_urls_.push_back(entry);
+
+  return *this;
+}
 
 NavigationPredictorKeyedService::Prediction::~Prediction() = default;