[Omnibox] Documents - check unity opt-in as prereq.

Falls back to sync if unified consent is not given, or hasn't been offered.

Also checks on offering search suggestions, since that seems logical.

Bug: 877850

Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I8daab9213c3a6b71b2bd8a1dd921973b51b7db8e
Reviewed-on: https://chromium-review.googlesource.com/1189702
Commit-Queue: Travis Skare <skare@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Reviewed-by: Thomas Tangl <tangltom@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590535}

chrome/browser/autocomplete/chrome_autocomplete_provider_client.cc

@@ -29,6 +29,7 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/unified_consent/unified_consent_service_factory.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "components/browser_sync/profile_sync_service.h"
@@ -37,6 +38,7 @@
 #include "components/omnibox/browser/autocomplete_match.h"
 #include "components/prefs/pref_service.h"
 #include "components/signin/core/browser/signin_manager.h"
+#include "components/unified_consent/unified_consent_service.h"
 #include "components/unified_consent/url_keyed_data_collection_consent_helper.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_service.h"
@@ -279,6 +281,19 @@ bool ChromeAutocompleteProviderClient::IsAuthenticated() const {
   return signin_manager != nullptr && signin_manager->IsAuthenticated();
 }
 
+bool ChromeAutocompleteProviderClient::IsUnifiedConsentGiven() const {
+  unified_consent::UnifiedConsentService* consent_service =
+      UnifiedConsentServiceFactory::GetForProfile(profile_);
+  return consent_service && consent_service->IsUnifiedConsentGiven();
+}
+
+bool ChromeAutocompleteProviderClient::IsSyncActive() const {
+  syncer::SyncService* sync =
+      ProfileSyncServiceFactory::GetInstance()->GetSyncServiceForBrowserContext(
+          profile_);
+  return sync && sync->IsSyncActive();
+}
+
 void ChromeAutocompleteProviderClient::Classify(
     const base::string16& text,
     bool prefer_keyword,

chrome/browser/autocomplete/chrome_autocomplete_provider_client.h

@@ -55,6 +55,8 @@ class ChromeAutocompleteProviderClient : public AutocompleteProviderClient {
   bool SearchSuggestEnabled() const override;
   bool IsPersonalizedUrlDataCollectionActive() const override;
   bool IsAuthenticated() const override;
+  bool IsUnifiedConsentGiven() const override;
+  bool IsSyncActive() const override;
   void Classify(
       const base::string16& text,
       bool prefer_keyword,

components/omnibox/browser/autocomplete_provider_client.h

@@ -106,6 +106,15 @@ class AutocompleteProviderClient {
   // This function returns true if the user is signed in.
   virtual bool IsAuthenticated() const = 0;
 
+  // Determines whether Unified Consent is on as a feature, and the user has
+  // accepted the bit. Note this is a subset of
+  // IsPersonalizedUrlDataCollectionActive() in that the user has not
+  // necessarily consented to share browsing data with Google.
+  virtual bool IsUnifiedConsentGiven() const = 0;
+
+  // Determines whether sync is enabled.
+  virtual bool IsSyncActive() const = 0;
+
   // Given some string |text| that the user wants to use for navigation,
   // determines how it should be interpreted.
   virtual void Classify(

components/omnibox/browser/document_provider.cc

@@ -110,24 +110,30 @@ void DocumentProvider::RegisterProfilePrefs(
 }
 
 bool DocumentProvider::IsDocumentProviderAllowed(
-    PrefService* prefs,
-    bool is_incognito,
-    bool is_authenticated,
-    const TemplateURLService* template_url_service) {
+    AutocompleteProviderClient* client) {
   // Feature must be on.
   if (!base::FeatureList::IsEnabled(omnibox::kDocumentProvider))
     return false;
 
+  // These may seem like search suggestions, so gate on that setting too.
+  if (!client->SearchSuggestEnabled())
+    return false;
+
   // Client-side toggle must be enabled.
-  if (!prefs->GetBoolean(omnibox::kDocumentSuggestEnabled))
+  if (!client->GetPrefs()->GetBoolean(omnibox::kDocumentSuggestEnabled))
     return false;
 
   // No incognito.
-  if (is_incognito)
+  if (client->IsOffTheRecord())
     return false;
 
-  // User must be signed in.
-  if (!is_authenticated)
+  // If the user opted into unity, we may proceed.
+  // Otherwise (either unity hasn't been offered or the not-yet button was
+  // clicked), we may check sync's status and proceed if active.
+  bool authenticated_and_syncing =
+      client->IsAuthenticated() &&
+      (client->IsUnifiedConsentGiven() || client->IsSyncActive());
+  if (!authenticated_and_syncing)
     return false;
 
   // We haven't received a server backoff signal.
@@ -137,6 +143,7 @@ bool DocumentProvider::IsDocumentProviderAllowed(
 
   // Google must be set as default search provider; we mix results which may
   // change placement.
+  auto* template_url_service = client->GetTemplateURLService();
   if (template_url_service == nullptr)
     return false;
   const TemplateURL* default_provider =
@@ -173,9 +180,9 @@ void DocumentProvider::Start(const AutocompleteInput& input,
   TRACE_EVENT0("omnibox", "DocumentProvider::Start");
   matches_.clear();
 
-  if (!IsDocumentProviderAllowed(client_->GetPrefs(), client_->IsOffTheRecord(),
-                                 client_->IsAuthenticated(),
-                                 client_->GetTemplateURLService())) {
+  // Perform various checks - feature is enabled, user is allowed to use the
+  // feature, we're not under backoff, etc.
+  if (!IsDocumentProviderAllowed(client_)) {
     return;
   }
 

components/omnibox/browser/document_provider.h

@@ -22,7 +22,6 @@
 
 class AutocompleteProviderListener;
 class AutocompleteProviderClient;
-class PrefService;
 
 namespace base {
 class Value;
@@ -58,6 +57,8 @@ class DocumentProvider : public AutocompleteProvider {
   FRIEND_TEST_ALL_PREFIXES(DocumentProviderTest, CheckFeatureBehindFlag);
   FRIEND_TEST_ALL_PREFIXES(DocumentProviderTest,
                            CheckFeaturePrerequisiteNoIncognito);
+  FRIEND_TEST_ALL_PREFIXES(DocumentProviderTest,
+                           CheckFeaturePrerequisiteNoConsentBit);
   FRIEND_TEST_ALL_PREFIXES(DocumentProviderTest,
                            CheckFeaturePrerequisiteClientSettingOff);
   FRIEND_TEST_ALL_PREFIXES(DocumentProviderTest,
@@ -77,13 +78,9 @@ class DocumentProvider : public AutocompleteProvider {
 
   ~DocumentProvider() override;
 
-  // Using this provider requires the user is signed-in and has Google set as
-  // default search engine.
-  bool IsDocumentProviderAllowed(
-      PrefService* prefs,
-      bool is_incognito,
-      bool is_authenticated,
-      const TemplateURLService* template_url_service);
+  // Determines whether the profile/session/window meet the feature
+  // prerequisites.
+  bool IsDocumentProviderAllowed(AutocompleteProviderClient* client);
 
   // Determines if the input is a URL, or is the start of the user entering one.
   // We avoid queries for these cases for quality and scaling reasons.

components/omnibox/browser/document_provider_unittest.cc

@@ -22,6 +22,8 @@
 
 namespace {
 
+using testing::Return;
+
 class FakeAutocompleteProviderClient : public MockAutocompleteProviderClient {
  public:
   FakeAutocompleteProviderClient()
@@ -93,67 +95,82 @@ void DocumentProviderTest::OnProviderUpdate(bool updated_matches) {
 }
 
 TEST_F(DocumentProviderTest, CheckFeatureBehindFlag) {
-  PrefService* fake_prefs = client_->GetPrefs();
-  TemplateURLService* template_url_service = client_->GetTemplateURLService();
-  bool is_incognito = false;
-  bool is_authenticated = true;
-
   base::test::ScopedFeatureList feature_list;
   feature_list.InitAndDisableFeature(omnibox::kDocumentProvider);
-  EXPECT_FALSE(provider_->IsDocumentProviderAllowed(
-      fake_prefs, is_incognito, is_authenticated, template_url_service));
+  EXPECT_FALSE(provider_->IsDocumentProviderAllowed(client_.get()));
 }
 
 TEST_F(DocumentProviderTest, CheckFeaturePrerequisiteNoIncognito) {
-  PrefService* fake_prefs = client_->GetPrefs();
-  TemplateURLService* template_url_service = client_->GetTemplateURLService();
-  bool is_incognito = false;
-  bool is_authenticated = true;
   base::test::ScopedFeatureList feature_list;
   feature_list.InitAndEnableFeature(omnibox::kDocumentProvider);
+  EXPECT_CALL(*client_.get(), SearchSuggestEnabled())
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*client_.get(), IsAuthenticated()).WillRepeatedly(Return(true));
+  EXPECT_CALL(*client_.get(), IsUnifiedConsentGiven())
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*client_.get(), IsOffTheRecord()).WillRepeatedly(Return(false));
 
   // Feature starts enabled.
-  EXPECT_TRUE(provider_->IsDocumentProviderAllowed(
-      fake_prefs, is_incognito, is_authenticated, template_url_service));
+  EXPECT_TRUE(provider_->IsDocumentProviderAllowed(client_.get()));
 
   // Feature should be disabled in incognito.
-  is_incognito = true;
-  EXPECT_FALSE(provider_->IsDocumentProviderAllowed(
-      fake_prefs, is_incognito, is_authenticated, template_url_service));
+  EXPECT_CALL(*client_.get(), IsOffTheRecord()).WillRepeatedly(Return(true));
+  EXPECT_FALSE(provider_->IsDocumentProviderAllowed(client_.get()));
+}
+
+TEST_F(DocumentProviderTest, CheckFeaturePrerequisiteNoConsentBit) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(omnibox::kDocumentProvider);
+  EXPECT_CALL(*client_.get(), SearchSuggestEnabled())
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*client_.get(), IsAuthenticated()).WillRepeatedly(Return(true));
+  EXPECT_CALL(*client_.get(), IsUnifiedConsentGiven())
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*client_.get(), IsOffTheRecord()).WillRepeatedly(Return(false));
+
+  // Feature starts enabled.
+  EXPECT_TRUE(provider_->IsDocumentProviderAllowed(client_.get()));
+
+  // Feature should be disabled without a consent bit.
+  EXPECT_CALL(*client_.get(), IsUnifiedConsentGiven()).WillOnce(Return(false));
+  EXPECT_FALSE(provider_->IsDocumentProviderAllowed(client_.get()));
 }
 
 TEST_F(DocumentProviderTest, CheckFeaturePrerequisiteClientSettingOff) {
-  PrefService* fake_prefs = client_->GetPrefs();
-  TemplateURLService* template_url_service = client_->GetTemplateURLService();
-  bool is_incognito = false;
-  bool is_authenticated = true;
   base::test::ScopedFeatureList feature_list;
   feature_list.InitAndEnableFeature(omnibox::kDocumentProvider);
+  EXPECT_CALL(*client_.get(), SearchSuggestEnabled())
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*client_.get(), IsAuthenticated()).WillRepeatedly(Return(true));
+  EXPECT_CALL(*client_.get(), IsUnifiedConsentGiven())
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*client_.get(), IsOffTheRecord()).WillRepeatedly(Return(false));
 
   // Feature starts enabled.
-  EXPECT_TRUE(provider_->IsDocumentProviderAllowed(
-      fake_prefs, is_incognito, is_authenticated, template_url_service));
+  EXPECT_TRUE(provider_->IsDocumentProviderAllowed(client_.get()));
 
   // Disabling toggle in chrome://settings should be respected.
+  PrefService* fake_prefs = client_->GetPrefs();
   fake_prefs->SetBoolean(omnibox::kDocumentSuggestEnabled, false);
-  EXPECT_FALSE(provider_->IsDocumentProviderAllowed(
-      fake_prefs, is_incognito, is_authenticated, template_url_service));
+  EXPECT_FALSE(provider_->IsDocumentProviderAllowed(client_.get()));
   fake_prefs->SetBoolean(omnibox::kDocumentSuggestEnabled, true);
 }
 
 TEST_F(DocumentProviderTest, CheckFeaturePrerequisiteDefaultSearch) {
-  PrefService* fake_prefs = client_->GetPrefs();
-  TemplateURLService* template_url_service = client_->GetTemplateURLService();
-  bool is_incognito = false;
-  bool is_authenticated = true;
   base::test::ScopedFeatureList feature_list;
   feature_list.InitAndEnableFeature(omnibox::kDocumentProvider);
+  EXPECT_CALL(*client_.get(), SearchSuggestEnabled())
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*client_.get(), IsAuthenticated()).WillRepeatedly(Return(true));
+  EXPECT_CALL(*client_.get(), IsUnifiedConsentGiven())
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*client_.get(), IsOffTheRecord()).WillRepeatedly(Return(false));
 
   // Feature starts enabled.
-  EXPECT_TRUE(provider_->IsDocumentProviderAllowed(
-      fake_prefs, is_incognito, is_authenticated, template_url_service));
+  EXPECT_TRUE(provider_->IsDocumentProviderAllowed(client_.get()));
 
   // Switching default search disables it.
+  TemplateURLService* template_url_service = client_->GetTemplateURLService();
   TemplateURLData data;
   data.SetShortName(base::ASCIIToUTF16("t"));
   data.SetURL("https://www.notgoogle.com/?q={searchTerms}");
@@ -162,29 +179,28 @@ TEST_F(DocumentProviderTest, CheckFeaturePrerequisiteDefaultSearch) {
       template_url_service->Add(std::make_unique<TemplateURL>(data));
   template_url_service->SetUserSelectedDefaultSearchProvider(
       new_default_provider);
-  EXPECT_FALSE(provider_->IsDocumentProviderAllowed(
-      fake_prefs, is_incognito, is_authenticated, template_url_service));
+  EXPECT_FALSE(provider_->IsDocumentProviderAllowed(client_.get()));
   template_url_service->SetUserSelectedDefaultSearchProvider(
       default_template_url_);
   template_url_service->Remove(new_default_provider);
 }
 
 TEST_F(DocumentProviderTest, CheckFeaturePrerequisiteServerBackoff) {
-  PrefService* fake_prefs = client_->GetPrefs();
-  TemplateURLService* template_url_service = client_->GetTemplateURLService();
-  bool is_incognito = false;
-  bool is_authenticated = true;
   base::test::ScopedFeatureList feature_list;
   feature_list.InitAndEnableFeature(omnibox::kDocumentProvider);
+  EXPECT_CALL(*client_.get(), SearchSuggestEnabled())
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*client_.get(), IsAuthenticated()).WillRepeatedly(Return(true));
+  EXPECT_CALL(*client_.get(), IsUnifiedConsentGiven())
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*client_.get(), IsOffTheRecord()).WillRepeatedly(Return(false));
 
   // Feature starts enabled.
-  EXPECT_TRUE(provider_->IsDocumentProviderAllowed(
-      fake_prefs, is_incognito, is_authenticated, template_url_service));
+  EXPECT_TRUE(provider_->IsDocumentProviderAllowed(client_.get()));
 
   // Server setting backoff flag disables it.
   provider_->backoff_for_session_ = true;
-  EXPECT_FALSE(provider_->IsDocumentProviderAllowed(
-      fake_prefs, is_incognito, is_authenticated, template_url_service));
+  EXPECT_FALSE(provider_->IsDocumentProviderAllowed(client_.get()));
   provider_->backoff_for_session_ = false;
 }
 

components/omnibox/browser/mock_autocomplete_provider_client.h

@@ -82,6 +82,9 @@ class MockAutocompleteProviderClient
   MOCK_CONST_METHOD0(SearchSuggestEnabled, bool());
   MOCK_CONST_METHOD0(IsPersonalizedUrlDataCollectionActive, bool());
   MOCK_CONST_METHOD0(IsAuthenticated, bool());
+  MOCK_CONST_METHOD0(IsUnifiedConsentGiven, bool());
+  MOCK_CONST_METHOD0(IsSyncActive, bool());
+
   MOCK_METHOD6(
       Classify,
       void(const base::string16& text,

ios/chrome/browser/autocomplete/BUILD.gn

@@ -36,6 +36,7 @@ source_set("autocomplete") {
     "//ios/chrome/browser/search_engines",
     "//ios/chrome/browser/signin",
     "//ios/chrome/browser/sync",
+    "//ios/chrome/browser/unified_consent",
     "//ios/public/provider/chrome/browser",
     "//ios/web",
     "//url",

ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.cc

@@ -12,6 +12,7 @@
 #include "components/prefs/pref_service.h"
 #include "components/signin/core/browser/signin_manager.h"
 #include "components/sync/driver/sync_service.h"
+#include "components/unified_consent/unified_consent_service.h"
 #include "components/unified_consent/url_keyed_data_collection_consent_helper.h"
 #include "ios/chrome/browser/application_context.h"
 #include "ios/chrome/browser/autocomplete/autocomplete_classifier_factory.h"
@@ -26,6 +27,7 @@
 #include "ios/chrome/browser/search_engines/template_url_service_factory.h"
 #include "ios/chrome/browser/signin/signin_manager_factory.h"
 #include "ios/chrome/browser/sync/profile_sync_service_factory.h"
+#include "ios/chrome/browser/unified_consent/unified_consent_service_factory.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 
 AutocompleteProviderClientImpl::AutocompleteProviderClientImpl(
@@ -169,6 +171,18 @@ bool AutocompleteProviderClientImpl::IsAuthenticated() const {
   return signin_manager != nullptr && signin_manager->IsAuthenticated();
 }
 
+bool AutocompleteProviderClientImpl::IsUnifiedConsentGiven() const {
+  unified_consent::UnifiedConsentService* consent_service =
+      UnifiedConsentServiceFactory::GetForBrowserState(browser_state_);
+  return consent_service && consent_service->IsUnifiedConsentGiven();
+}
+
+bool AutocompleteProviderClientImpl::IsSyncActive() const {
+  syncer::SyncService* sync =
+      ProfileSyncServiceFactory::GetForBrowserState(browser_state_);
+  return sync && sync->IsSyncActive();
+}
+
 void AutocompleteProviderClientImpl::Classify(
     const base::string16& text,
     bool prefer_keyword,

ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.h

@@ -56,6 +56,8 @@ class AutocompleteProviderClientImpl : public AutocompleteProviderClient {
   bool SearchSuggestEnabled() const override;
   bool IsPersonalizedUrlDataCollectionActive() const override;
   bool IsAuthenticated() const override;
+  bool IsUnifiedConsentGiven() const override;
+  bool IsSyncActive() const override;
   void Classify(
       const base::string16& text,
       bool prefer_keyword,