Revert "[omnibox] improve on device head suggest incognito flags."

This reverts commit 77add479.

Reason for revert: This cl introduces consistent test failures https://ci.chromium.org/p/chromium/builders/ci/Linux%20TSan%20Tests/46230

Original change's description:
> [omnibox] improve on device head suggest incognito flags.
> 
> 1. Allows to enable on device head suggest in incognito w/o enabling it
> in non incognito first.
> 2. Removes request delay in incognito since there is no server
> suggestion which we need to cope with.
> 
> Bug: 925072
> Change-Id: I344ecc2c620cb95c3b5f7c929cac0d9287b3c05b
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1881978
> Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
> Reviewed-by: Tommy Li <tommycli@chromium.org>
> Commit-Queue: Ce Chen <cch@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#710127}

TBR=tommycli@chromium.org,jdonnelly@chromium.org,cch@chromium.org

Change-Id: I7feaf59d4f0c16afb89e6048e622e4c5e294afd5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 925072
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1886333Reviewed-by: Kamila Hasanbega <hkamila@chromium.org>
Commit-Queue: Kamila Hasanbega <hkamila@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710287}

components/omnibox/browser/omnibox_field_trial.cc

@@ -681,6 +681,11 @@ bool OmniboxFieldTrial::IsMaxURLMatchesFeatureEnabled() {
   return base::FeatureList::IsEnabled(omnibox::kOmniboxMaxURLMatches);
 }
 
+bool OmniboxFieldTrial::IsOnDeviceHeadProviderEnabledForIncognito() {
+  return base::GetFieldTrialParamByFeatureAsBool(omnibox::kOnDeviceHeadProvider,
+                                                 "EnableForIncongnito", false);
+}
+
 const char OmniboxFieldTrial::kBundledExperimentFieldTrialName[] =
     "OmniboxBundledExperimentV1";
 const char OmniboxFieldTrial::kDisableProvidersRule[] = "DisableProviders";

components/omnibox/browser/omnibox_field_trial.h

@@ -414,6 +414,9 @@ bool IsGroupSuggestionsBySearchVsUrlFeatureEnabled();
 // is enabled.
 bool IsMaxURLMatchesFeatureEnabled();
 
+// Returns whether on device head provider is enabled for incognito mode.
+bool IsOnDeviceHeadProviderEnabledForIncognito();
+
 // ---------------------------------------------------------
 // Clipboard URL suggestions:
 

components/omnibox/browser/on_device_head_provider.cc

@@ -108,20 +108,11 @@ bool OnDeviceHeadProvider::IsOnDeviceHeadProviderAllowed(
   if (!client()->SearchSuggestEnabled())
     return false;
 
-  // This flag specifies whether we should serve incognito or non incognito
-  // request, value can be:
-  // 1. incognito-not-allowed (or empty string): only serve non incognito
-  //    request; this is the default behavior;
-  // 2. incognito-only: only serve incognito request;
-  // 3. always-serve: always serve regardless of incognito.
-  const std::string mode = base::GetFieldTrialParamValueByFeature(
-      omnibox::kOnDeviceHeadProvider, "IncognitoServeMode");
-  if (mode != "always-serve") {
-    if (client()->IsOffTheRecord() && mode != "incognito-only")
-      return false;
-    if (!client()->IsOffTheRecord() && mode == "incognito-only")
-      return false;
-  }
+  // Make sure user is not in incognito, unless on device head provider is
+  // enabled for incognito.
+  if (client()->IsOffTheRecord() &&
+      !OmniboxFieldTrial::IsOnDeviceHeadProviderEnabledForIncognito())
+    return false;
 
   // Reject on focus request.
   if (input.from_omnibox_focus())
@@ -165,14 +156,8 @@ void OnDeviceHeadProvider::Start(const AutocompleteInput& input,
     // from server, if we issue both requests simultaneously.
     // Therefore, we might want to delay the On Device suggest requests (and
     // also apply a timeout to search default loader) to mitigate this issue.
-    // Note this delay is not needed for incognito where server suggestion is
-    // not served.
-    int delay = 0;
-    if (!client()->IsOffTheRecord()) {
-      delay = base::GetFieldTrialParamByFeatureAsInt(
-          omnibox::kOnDeviceHeadProvider, "DelayOnDeviceHeadSuggestRequestMs",
-          0);
-    }
+    int delay = base::GetFieldTrialParamByFeatureAsInt(
+        omnibox::kOnDeviceHeadProvider, "DelayOnDeviceHeadSuggestRequestMs", 0);
     task_runner_->PostDelayedTask(
         FROM_HERE,
         base::BindOnce(&OnDeviceHeadProvider::DoSearch,

components/omnibox/browser/on_device_head_provider_unittest.cc

@@ -8,7 +8,6 @@
 #include "base/path_service.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/test/scoped_feature_list.h"
 #include "base/test/task_environment.h"
 #include "build/build_config.h"
 #include "components/omnibox/browser/autocomplete_input.h"
@@ -16,7 +15,6 @@
 #include "components/omnibox/browser/fake_autocomplete_provider_client.h"
 #include "components/omnibox/browser/on_device_head_serving.h"
 #include "components/omnibox/browser/test_scheme_classifier.h"
-#include "components/omnibox/common/omnibox_features.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -77,7 +75,7 @@ TEST_F(OnDeviceHeadProviderTest, ServingInstanceNotCreated) {
   input.set_want_asynchronous_matches(true);
   ResetServingInstance();
 
-  EXPECT_CALL(*client_.get(), IsOffTheRecord()).WillRepeatedly(Return(false));
+  EXPECT_CALL(*client_.get(), IsOffTheRecord()).WillOnce(Return(false));
   EXPECT_CALL(*client_.get(), SearchSuggestEnabled()).WillOnce(Return(true));
 
   provider_->Start(input, false);
@@ -102,61 +100,21 @@ TEST_F(OnDeviceHeadProviderTest, RejectSynchronousRequest) {
   EXPECT_TRUE(provider_->done());
 }
 
-TEST_F(OnDeviceHeadProviderTest, TestIncognito) {
+TEST_F(OnDeviceHeadProviderTest, RejectIncognito) {
   AutocompleteInput input(base::UTF8ToUTF16("M"),
                           metrics::OmniboxEventProto::OTHER,
                           TestSchemeClassifier());
   input.set_want_asynchronous_matches(true);
 
-  EXPECT_CALL(*client_.get(), IsOffTheRecord()).WillRepeatedly(Return(true));
-  EXPECT_CALL(*client_.get(), SearchSuggestEnabled())
-      .WillRepeatedly(Return(true));
-
-  // By default incognito request will be rejected.
-  {
-    provider_->Start(input, false);
-    if (!provider_->done())
-      task_environment_.RunUntilIdle();
-
-    EXPECT_TRUE(provider_->matches().empty());
-    EXPECT_TRUE(provider_->done());
-  }
+  EXPECT_CALL(*client_.get(), IsOffTheRecord()).WillOnce(Return(true));
+  EXPECT_CALL(*client_.get(), SearchSuggestEnabled()).WillOnce(Return(true));
 
-  // Now enable for incognito only.
-  {
-    base::test::ScopedFeatureList feature_list;
-    feature_list.InitAndEnableFeatureWithParameters(
-        omnibox::kOnDeviceHeadProvider,
-        {{"IncognitoServeMode", "incognito-only"}});
-
-    provider_->Start(input, false);
-    if (!provider_->done())
-      task_environment_.RunUntilIdle();
-
-    EXPECT_TRUE(provider_->done());
-    ASSERT_EQ(3U, provider_->matches().size());
-    EXPECT_EQ(base::UTF8ToUTF16("maps"), provider_->matches()[0].contents);
-    EXPECT_EQ(base::UTF8ToUTF16("mail"), provider_->matches()[1].contents);
-    EXPECT_EQ(base::UTF8ToUTF16("map"), provider_->matches()[2].contents);
-  }
+  provider_->Start(input, false);
+  if (!provider_->done())
+    task_environment_.RunUntilIdle();
 
-  // Test "always-serve" mode.
-  {
-    base::test::ScopedFeatureList feature_list;
-    feature_list.InitAndEnableFeatureWithParameters(
-        omnibox::kOnDeviceHeadProvider,
-        {{"IncognitoServeMode", "always-serve"}});
-
-    provider_->Start(input, false);
-    if (!provider_->done())
-      task_environment_.RunUntilIdle();
-
-    EXPECT_TRUE(provider_->done());
-    ASSERT_EQ(3U, provider_->matches().size());
-    EXPECT_EQ(base::UTF8ToUTF16("maps"), provider_->matches()[0].contents);
-    EXPECT_EQ(base::UTF8ToUTF16("mail"), provider_->matches()[1].contents);
-    EXPECT_EQ(base::UTF8ToUTF16("map"), provider_->matches()[2].contents);
-  }
+  EXPECT_TRUE(provider_->matches().empty());
+  EXPECT_TRUE(provider_->done());
 }
 
 TEST_F(OnDeviceHeadProviderTest, RejectOnFocusRequest) {
@@ -166,7 +124,7 @@ TEST_F(OnDeviceHeadProviderTest, RejectOnFocusRequest) {
   input.set_want_asynchronous_matches(true);
   input.set_from_omnibox_focus(true);
 
-  EXPECT_CALL(*client_.get(), IsOffTheRecord()).WillRepeatedly(Return(false));
+  EXPECT_CALL(*client_.get(), IsOffTheRecord()).WillOnce(Return(false));
   EXPECT_CALL(*client_.get(), SearchSuggestEnabled()).WillOnce(Return(true));
 
   provider_->Start(input, false);
@@ -183,7 +141,7 @@ TEST_F(OnDeviceHeadProviderTest, NoMatches) {
                           TestSchemeClassifier());
   input.set_want_asynchronous_matches(true);
 
-  EXPECT_CALL(*client_.get(), IsOffTheRecord()).WillRepeatedly(Return(false));
+  EXPECT_CALL(*client_.get(), IsOffTheRecord()).WillOnce(Return(false));
   EXPECT_CALL(*client_.get(), SearchSuggestEnabled()).WillOnce(Return(true));
 
   provider_->Start(input, false);
@@ -200,7 +158,7 @@ TEST_F(OnDeviceHeadProviderTest, HasMatches) {
                           TestSchemeClassifier());
   input.set_want_asynchronous_matches(true);
 
-  EXPECT_CALL(*client_.get(), IsOffTheRecord()).WillRepeatedly(Return(false));
+  EXPECT_CALL(*client_.get(), IsOffTheRecord()).WillOnce(Return(false));
   EXPECT_CALL(*client_.get(), SearchSuggestEnabled()).WillOnce(Return(true));
 
   provider_->Start(input, false);