BASELINE: Update Chromium to 71.0.3578.93

Change-Id: I6a32086c33670e1b033f8b10e6bf1fd4da1d105d
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>

6 files changed, 113 insertions, 39 deletions

diff --git a/chromium/components/ukm/observers/sync_disable_observer.cc b/chromium/components/ukm/observers/sync_disable_observer.cc
index fb364a8532b..0309c136113 100644
--- a/chromium/components/ukm/observers/sync_disable_observer.cc
+++ b/chromium/components/ukm/observers/sync_disable_observer.cc
@@ -4,6 +4,9 @@
 
 #include "components/ukm/observers/sync_disable_observer.h"
 
+#include <memory>
+#include <utility>
+
 #include "base/feature_list.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/stl_util.h"
@@ -18,6 +21,10 @@ namespace ukm {
 
 const base::Feature kUkmCheckAuthErrorFeature{"UkmCheckAuthError",
                                               base::FEATURE_ENABLED_BY_DEFAULT};
+const base::Feature kUkmCheckEnabledForDatatypes{
+    "UkmCheckEnabledForDatatypes", base::FEATURE_ENABLED_BY_DEFAULT};
+const base::Feature kUkmPurgingOnConnection{"UkmPurgingOnConnection",
+                                            base::FEATURE_DISABLED_BY_DEFAULT};
 
 namespace {
 
@@ -74,13 +81,37 @@ SyncDisableObserver::SyncState SyncDisableObserver::GetSyncState(
     UrlKeyedDataCollectionConsentHelper* consent_helper) {
   syncer::SyncTokenStatus status = sync_service->GetSyncTokenStatus();
   SyncState state;
-  state.history_enabled = sync_service->GetPreferredDataTypes().Has(
-      syncer::HISTORY_DELETE_DIRECTIVES);
-  state.extensions_enabled =
-      sync_service->GetPreferredDataTypes().Has(syncer::EXTENSIONS);
+
+  // For the following two settings, we want them to match the state of a user
+  // having history/extensions enabled in their Sync settings. Using it to track
+  // active connections here is undesirable as changes in these states trigger
+  // data purges.
+  if (base::FeatureList::IsEnabled(kUkmCheckEnabledForDatatypes)) {
+    state.history_enabled = sync_service->GetPreferredDataTypes().Has(
+                                syncer::HISTORY_DELETE_DIRECTIVES) &&
+                            sync_service->IsSyncFeatureEnabled();
+
+    state.extensions_enabled =
+        sync_service->GetPreferredDataTypes().Has(syncer::EXTENSIONS) &&
+        sync_service->IsSyncFeatureEnabled();
+  } else {
+    // If kUkmCheckEnabledForDatatypes is disabled, use legacy settings.
+    // TODO(rkaplow): Clean this up after crbug.com/906609.
+    state.history_enabled = sync_service->GetPreferredDataTypes().Has(
+        syncer::HISTORY_DELETE_DIRECTIVES);
+
+    state.extensions_enabled =
+        sync_service->GetPreferredDataTypes().Has(syncer::EXTENSIONS);
+  }
+
   state.initialized = sync_service->IsEngineInitialized();
+
+  // We use CONNECTION_OK here as an auth error can be used in the sync
+  // paused state. Therefore we need to be more direct and check CONNECTION_OK
+  // as opposed to using something like IsSyncFeatureActive().
   state.connected = !base::FeatureList::IsEnabled(kUkmCheckAuthErrorFeature) ||
                     status.connection_status == syncer::CONNECTION_OK;
+
   state.passphrase_protected =
       state.initialized && sync_service->IsUsingSecondaryPassphrase();
   if (consent_helper) {
@@ -198,8 +229,35 @@ void SyncDisableObserver::UpdateSyncState(
              DataCollectionState::kIgnored ||
          consent_helper);
   SyncDisableObserver::SyncState state = GetSyncState(sync, consent_helper);
+
   // Trigger a purge if sync state no longer allows UKM.
-  bool must_purge = previous_state.AllowsUkm() && !state.AllowsUkm();
+  // TODO(rkaplow): Clean this up once crbug.com/891777 is resolved.
+  bool must_purge;
+
+  // If unified_consent is used, we keep the logic introduced in
+  // http://crrev.com/c/1152744. Otherwise, if the kUkmPurgingOnConnection
+  // feature is enabled, we still use that logic.
+  if (unified_consent::IsUnifiedConsentFeatureEnabled() ||
+      base::FeatureList::IsEnabled(kUkmPurgingOnConnection)) {
+    // Purge using AllowsUkm which includes connected status.
+    must_purge = previous_state.AllowsUkm() && !state.AllowsUkm();
+  } else {
+    // Use the previous logic to investigate crbug.com/891777.
+    must_purge =
+        // Trigger a purge if history sync was disabled.
+        (previous_state.history_enabled && !state.history_enabled) ||
+        // Trigger a purge if engine has become disabled.
+        (previous_state.initialized && !state.initialized) ||
+        // Trigger a purge if the user added a passphrase.  Since we can't
+        // detect the use of a passphrase while the engine is not initialized,
+        // we may miss the transition if the user adds a passphrase in this
+        // state.
+        (previous_state.initialized && state.initialized &&
+         !previous_state.passphrase_protected && state.passphrase_protected);
+  }
+
+  UMA_HISTOGRAM_BOOLEAN("UKM.SyncDisable.Purge", must_purge);
+
   previous_states_[sync] = state;
   UpdateAllProfileEnabled(must_purge);
 }
diff --git a/chromium/components/ukm/observers/sync_disable_observer_unittest.cc b/chromium/components/ukm/observers/sync_disable_observer_unittest.cc
index cf262683cf0..6ecefcc5e00 100644
--- a/chromium/components/ukm/observers/sync_disable_observer_unittest.cc
+++ b/chromium/components/ukm/observers/sync_disable_observer_unittest.cc
@@ -5,8 +5,8 @@
 #include "components/ukm/observers/sync_disable_observer.h"
 
 #include "base/observer_list.h"
-#include "components/sync/driver/fake_sync_service.h"
 #include "components/sync/driver/sync_token_status.h"
+#include "components/sync/driver/test_sync_service.h"
 #include "components/sync/engine/connection_status.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "components/unified_consent/pref_names.h"
@@ -21,18 +21,18 @@ namespace ukm {
 
 namespace {
 
-class MockSyncService : public syncer::FakeSyncService {
+class MockSyncService : public syncer::TestSyncService {
  public:
-  MockSyncService() {}
+  MockSyncService() { SetTransportState(TransportState::INITIALIZING); }
   ~MockSyncService() override { Shutdown(); }
 
   void SetStatus(bool has_passphrase, bool history_enabled) {
-    initialized_ = true;
-    has_passphrase_ = has_passphrase;
-    preferred_data_types_ =
+    SetTransportState(TransportState::ACTIVE);
+    SetCustomPassphraseEnabled(has_passphrase);
+    SetPreferredDataTypes(
         history_enabled
             ? syncer::ModelTypeSet(syncer::HISTORY_DELETE_DIRECTIVES)
-            : syncer::ModelTypeSet();
+            : syncer::ModelTypeSet());
     NotifyObserversOfStateChanged();
   }
 
@@ -54,30 +54,20 @@ class MockSyncService : public syncer::FakeSyncService {
   }
 
  private:
-  // syncer::FakeSyncService:
+  // syncer::TestSyncService:
   void AddObserver(syncer::SyncServiceObserver* observer) override {
     observers_.AddObserver(observer);
   }
   void RemoveObserver(syncer::SyncServiceObserver* observer) override {
     observers_.RemoveObserver(observer);
   }
-  TransportState GetTransportState() const override {
-    return initialized_ ? TransportState::ACTIVE : TransportState::INITIALIZING;
-  }
-  bool IsUsingSecondaryPassphrase() const override { return has_passphrase_; }
-  syncer::ModelTypeSet GetPreferredDataTypes() const override {
-    return preferred_data_types_;
-  }
   syncer::SyncTokenStatus GetSyncTokenStatus() const override {
     syncer::SyncTokenStatus status;
     status.connection_status = connection_status_;
     return status;
   }
 
-  bool initialized_ = false;
-  bool has_passphrase_ = false;
   syncer::ConnectionStatus connection_status_ = syncer::CONNECTION_OK;
-  syncer::ModelTypeSet preferred_data_types_;
 
   // The list of observers of the SyncService state.
   base::ObserverList<syncer::SyncServiceObserver>::Unchecked observers_;
diff --git a/chromium/components/ukm/test_ukm_recorder.cc b/chromium/components/ukm/test_ukm_recorder.cc
index 445348f9a48..bdc151b1bd3 100644
--- a/chromium/components/ukm/test_ukm_recorder.cc
+++ b/chromium/components/ukm/test_ukm_recorder.cc
@@ -56,6 +56,14 @@ bool TestUkmRecorder::ShouldRestrictToWhitelistedEntries() const {
   return false;
 }
 
+void TestUkmRecorder::AddEntry(mojom::UkmEntryPtr entry) {
+  const bool should_run_callback =
+      on_add_entry_ && entry && entry_hash_to_wait_for_ == entry->event_hash;
+  UkmRecorderImpl::AddEntry(std::move(entry));
+  if (should_run_callback)
+    std::move(on_add_entry_).Run();
+}
+
 const UkmSource* TestUkmRecorder::GetSourceForSourceId(
     SourceId source_id) const {
   const UkmSource* source = nullptr;
@@ -68,6 +76,12 @@ const UkmSource* TestUkmRecorder::GetSourceForSourceId(
   return source;
 }
 
+void TestUkmRecorder::SetOnAddEntryCallback(base::StringPiece entry_name,
+                                            base::OnceClosure on_add_entry) {
+  on_add_entry_ = std::move(on_add_entry);
+  entry_hash_to_wait_for_ = base::HashMetricName(entry_name);
+}
+
 std::vector<const mojom::UkmEntry*> TestUkmRecorder::GetEntriesByName(
     base::StringPiece entry_name) const {
   uint64_t hash = base::HashMetricName(entry_name);
diff --git a/chromium/components/ukm/test_ukm_recorder.h b/chromium/components/ukm/test_ukm_recorder.h
index 6950c3c5c31..c79206c1453 100644
--- a/chromium/components/ukm/test_ukm_recorder.h
+++ b/chromium/components/ukm/test_ukm_recorder.h
@@ -31,6 +31,8 @@ class TestUkmRecorder : public UkmRecorderImpl {
   bool ShouldRestrictToWhitelistedSourceIds() const override;
   bool ShouldRestrictToWhitelistedEntries() const override;
 
+  void AddEntry(mojom::UkmEntryPtr entry) override;
+
   size_t sources_count() const { return sources().size(); }
 
   size_t entries_count() const { return entries().size(); }
@@ -47,6 +49,10 @@ class TestUkmRecorder : public UkmRecorderImpl {
   // Gets UkmSource data for a single SourceId.
   const UkmSource* GetSourceForSourceId(ukm::SourceId source_id) const;
 
+  // Sets a callback that will be called when recording an entry for entry name.
+  void SetOnAddEntryCallback(base::StringPiece entry_name,
+                             base::OnceClosure on_add_entry);
+
   // Gets all of the entries recorded for entry name.
   std::vector<const mojom::UkmEntry*> GetEntriesByName(
       base::StringPiece entry_name) const;
@@ -75,6 +81,9 @@ class TestUkmRecorder : public UkmRecorderImpl {
                                        base::StringPiece metric_name);
 
  private:
+  uint64_t entry_hash_to_wait_for_ = 0;
+  base::OnceClosure on_add_entry_;
+
   DISALLOW_COPY_AND_ASSIGN(TestUkmRecorder);
 };
 
diff --git a/chromium/components/ukm/ukm_recorder_impl.cc b/chromium/components/ukm/ukm_recorder_impl.cc
index 2f0dc35c6d8..5257c81b66f 100644
--- a/chromium/components/ukm/ukm_recorder_impl.cc
+++ b/chromium/components/ukm/ukm_recorder_impl.cc
@@ -239,6 +239,8 @@ void UkmRecorderImpl::EnableRecording(bool extensions) {
 
 void UkmRecorderImpl::DisableRecording() {
   DVLOG(1) << "UkmRecorderImpl::DisableRecording";
+  if (recording_enabled_)
+    recording_is_continuous_ = false;
   recording_enabled_ = false;
   extensions_enabled_ = false;
 }
@@ -250,6 +252,7 @@ void UkmRecorderImpl::DisableSamplingForTesting() {
 void UkmRecorderImpl::Purge() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   recordings_.Reset();
+  recording_is_continuous_ = false;
 }
 
 void UkmRecorderImpl::SetIsWebstoreExtensionCallback(
@@ -359,6 +362,9 @@ void UkmRecorderImpl::StoreRecordingsInReport(Report* report) {
   recordings_.entries.clear();
   recordings_.event_aggregations.clear();
 
+  report->set_is_continuous(recording_is_continuous_);
+  recording_is_continuous_ = true;
+
   // Keep at most |max_kept_sources|, prioritizing most-recent entries (by
   // creation time).
   const size_t max_kept_sources = GetMaxKeptSources();
@@ -396,19 +402,14 @@ bool UkmRecorderImpl::ShouldRestrictToWhitelistedEntries() const {
 void UkmRecorderImpl::UpdateSourceURL(SourceId source_id,
                                       const GURL& unsanitized_url) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  const GURL sanitized_url = SanitizeURL(unsanitized_url);
-  if (!ShouldRecordUrl(source_id, sanitized_url))
-    return;
-
-  // TODO(csharrison): These checks can probably move to ShouldRecordUrl.
 
   if (base::ContainsKey(recordings_.sources, source_id))
     return;
 
-  if (recordings_.sources.size() >= GetMaxSources()) {
-    RecordDroppedSource(DroppedDataReason::MAX_HIT);
+  const GURL sanitized_url = SanitizeURL(unsanitized_url);
+  if (!ShouldRecordUrl(source_id, sanitized_url))
     return;
-  }
+
   RecordSource(std::make_unique<UkmSource>(source_id, sanitized_url));
 }
 
@@ -424,6 +425,7 @@ void UkmRecorderImpl::RecordNavigation(
     SourceId source_id,
     const UkmSource::NavigationData& unsanitized_navigation_data) {
   DCHECK(GetSourceIdType(source_id) == SourceIdType::NAVIGATION_ID);
+  DCHECK(!base::ContainsKey(recordings_.sources, source_id));
   // TODO(csharrison): Consider changing this behavior so the Source isn't event
   // recorded at all if the final URL in |unsanitized_navigation_data| should
   // not be recorded.
@@ -441,12 +443,6 @@ void UkmRecorderImpl::RecordNavigation(
 
   UkmSource::NavigationData sanitized_navigation_data =
       unsanitized_navigation_data.CopyWithSanitizedUrls(urls);
-  // TODO(csharrison): This check can probably move to ShouldRecordUrl.
-  DCHECK(!base::ContainsKey(recordings_.sources, source_id));
-  if (recordings_.sources.size() >= GetMaxSources()) {
-    RecordDroppedSource(DroppedDataReason::MAX_HIT);
-    return;
-  }
   RecordSource(
       std::make_unique<UkmSource>(source_id, sanitized_navigation_data));
 }
@@ -458,6 +454,11 @@ bool UkmRecorderImpl::ShouldRecordUrl(SourceId source_id,
     return false;
   }
 
+  if (recordings_.sources.size() >= GetMaxSources()) {
+    RecordDroppedSource(DroppedDataReason::MAX_HIT);
+    return false;
+  }
+
   if (ShouldRestrictToWhitelistedSourceIds() &&
       !IsWhitelistedSourceId(source_id)) {
     RecordDroppedSource(DroppedDataReason::NOT_WHITELISTED);
diff --git a/chromium/components/ukm/ukm_recorder_impl.h b/chromium/components/ukm/ukm_recorder_impl.h
index 7f3084f7b6e..2179f81dfbd 100644
--- a/chromium/components/ukm/ukm_recorder_impl.h
+++ b/chromium/components/ukm/ukm_recorder_impl.h
@@ -82,6 +82,7 @@ class UkmRecorderImpl : public UkmRecorder {
   }
 
   // UkmRecorder:
+  void AddEntry(mojom::UkmEntryPtr entry) override;
   void UpdateSourceURL(SourceId source_id, const GURL& url) override;
   void UpdateAppURL(SourceId source_id, const GURL& url) override;
   void RecordNavigation(
@@ -126,8 +127,6 @@ class UkmRecorderImpl : public UkmRecorder {
 
   void RecordSource(std::unique_ptr<UkmSource> source);
 
-  void AddEntry(mojom::UkmEntryPtr entry) override;
-
   // Load sampling configurations from field-trial information.
   void LoadExperimentSamplingInfo();
 
@@ -137,6 +136,9 @@ class UkmRecorderImpl : public UkmRecorder {
   // Indicates whether recording is enabled for extensions.
   bool extensions_enabled_ = false;
 
+  // Indicates whether recording continuity has been broken since last report.
+  bool recording_is_continuous_ = true;
+
   // Indicates if sampling has been enabled.
   bool sampling_enabled_ = true;