[omnibox]: Include variation header for drive requests.
[email protected]
(cherry picked from commit a755a70df587e64131cf8f89fa2b0a2251eab33a)
Bug: 992509
Change-Id: I22e738a9a4c22954d310a66b28caf69224baadcb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1746123
Reviewed-by: Marc Treib <[email protected]>
Reviewed-by: Justin Donnelly (OOO until Aug 19) <[email protected]>
Commit-Queue: manuk hovanesian <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#685984}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1751909
Reviewed-by: manuk hovanesian <[email protected]>
Cr-Commit-Position: refs/branch-heads/3865@{#367}
Cr-Branched-From: 0cdcc6158160790658d1f033d3db873603250124-refs/heads/master@{#681094}
diff --git a/components/omnibox/browser/BUILD.gn b/components/omnibox/browser/BUILD.gn
index 1d0ab1a5..58cb319 100644
--- a/components/omnibox/browser/BUILD.gn
+++ b/components/omnibox/browser/BUILD.gn
@@ -386,6 +386,7 @@
"builtin_provider_unittest.cc",
"clipboard_provider_unittest.cc",
"document_provider_unittest.cc",
+ "document_suggestions_service_unittest.cc",
"favicon_cache_unittest.cc",
"history_provider_unittest.cc",
"history_quick_provider_unittest.cc",
@@ -432,9 +433,12 @@
"//components/search",
"//components/search_engines",
"//components/sessions",
+ "//components/signin/public/identity_manager:test_support",
"//components/strings",
+ "//components/sync_preferences:test_support",
"//components/url_formatter",
"//components/variations",
+ "//components/variations/net:net",
"//services/network:test_support",
"//sql",
"//sql:test_support",
diff --git a/components/omnibox/browser/DEPS b/components/omnibox/browser/DEPS
index 55868a6..aaf6678 100644
--- a/components/omnibox/browser/DEPS
+++ b/components/omnibox/browser/DEPS
@@ -2,8 +2,8 @@
"+components/bookmarks/browser",
"+components/bookmarks/test",
"+components/component_updater",
- "+components/favicon_base",
"+components/favicon/core",
+ "+components/favicon_base",
"+components/grit",
"+components/history/core/browser",
"+components/history/core/test",
@@ -20,8 +20,9 @@
"+components/security_state",
"+components/sessions",
"+components/signin/public",
- "+components/sync",
"+components/strings/grit/components_strings.h",
+ "+components/sync",
+ "+components/sync_preferences",
"+components/url_formatter",
"+components/variations",
"+components/vector_icons",
diff --git a/components/omnibox/browser/document_provider.cc b/components/omnibox/browser/document_provider.cc
index 863f8e29..6d37d21a 100644
--- a/components/omnibox/browser/document_provider.cc
+++ b/components/omnibox/browser/document_provider.cc
@@ -388,16 +388,10 @@
input_ = input;
- // Create a request for suggestions, routing completion to
- base::BindOnce(&DocumentProvider::OnDocumentSuggestionsLoaderAvailable,
- weak_ptr_factory_.GetWeakPtr()),
- base::BindOnce(&DocumentProvider::OnURLLoadComplete,
- base::Unretained(this) /* own SimpleURLLoader */);
-
done_ = false; // Set true in callbacks.
client_->GetDocumentSuggestionsService(/*create_if_necessary=*/true)
->CreateDocumentSuggestionsRequest(
- input.text(), client_->GetTemplateURLService(),
+ input.text(), client_->IsOffTheRecord(),
base::BindOnce(
&DocumentProvider::OnDocumentSuggestionsLoaderAvailable,
weak_ptr_factory_.GetWeakPtr()),
diff --git a/components/omnibox/browser/document_suggestions_service.cc b/components/omnibox/browser/document_suggestions_service.cc
index 9881fee..7b8c5f7 100644
--- a/components/omnibox/browser/document_suggestions_service.cc
+++ b/components/omnibox/browser/document_suggestions_service.cc
@@ -12,13 +12,12 @@
#include "base/json/json_writer.h"
#include "base/memory/scoped_refptr.h"
#include "base/metrics/field_trial_params.h"
-#include "base/strings/stringprintf.h"
#include "base/values.h"
#include "components/omnibox/browser/document_provider.h"
#include "components/omnibox/common/omnibox_features.h"
-#include "components/search_engines/template_url_service.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "components/signin/public/identity_manager/primary_account_access_token_fetcher.h"
+#include "components/variations/net/variations_http_headers.h"
#include "components/variations/variations_associated_data.h"
#include "net/base/load_flags.h"
#include "net/traffic_annotation/network_traffic_annotation.h"
@@ -72,7 +71,7 @@
void DocumentSuggestionsService::CreateDocumentSuggestionsRequest(
const base::string16& query,
- const TemplateURLService* template_url_service,
+ bool is_incognito,
StartCallback start_callback,
CompletionCallback completion_callback) {
std::string endpoint = base::GetFieldTrialParamValueByFeature(
@@ -111,6 +110,14 @@
request->method = "POST";
std::string request_body = BuildDocumentSuggestionRequest(query);
request->load_flags = net::LOAD_DO_NOT_SAVE_COOKIES;
+ // It is expected that the user is signed in here. But we only care about
+ // experiment IDs from the variations server, which do not require the
+ // signed-in version of this method.
+ variations::AppendVariationsHeaderUnknownSignedIn(
+ request->url,
+ is_incognito ? variations::InIncognito::kYes
+ : variations::InIncognito::kNo,
+ request.get());
// Create and fetch an OAuth2 token.
std::string scope = "https://www.googleapis.com/auth/cloud_search.query";
diff --git a/components/omnibox/browser/document_suggestions_service.h b/components/omnibox/browser/document_suggestions_service.h
index 680ba2b..8fda040b 100644
--- a/components/omnibox/browser/document_suggestions_service.h
+++ b/components/omnibox/browser/document_suggestions_service.h
@@ -22,7 +22,6 @@
} // namespace signin
class GoogleServiceAuthError;
-class TemplateURLService;
// A service to fetch suggestions from a remote endpoint given a URL.
class DocumentSuggestionsService : public KeyedService {
@@ -43,11 +42,10 @@
// Creates and starts a document suggestion request for |query|.
// May obtain an OAuth2 token for the signed-in user.
- void CreateDocumentSuggestionsRequest(
- const base::string16& query,
- const TemplateURLService* template_url_service,
- StartCallback start_callback,
- CompletionCallback completion_callback);
+ void CreateDocumentSuggestionsRequest(const base::string16& query,
+ bool is_incognito,
+ StartCallback start_callback,
+ CompletionCallback completion_callback);
// Advises the service to stop any process that creates a suggestion request.
void StopCreatingDocumentSuggestionsRequest();
diff --git a/components/omnibox/browser/document_suggestions_service_unittest.cc b/components/omnibox/browser/document_suggestions_service_unittest.cc
new file mode 100644
index 0000000..e0464f3a
--- /dev/null
+++ b/components/omnibox/browser/document_suggestions_service_unittest.cc
@@ -0,0 +1,92 @@
+// Copyright 2019 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/omnibox/browser/document_suggestions_service.h"
+
+#include "base/bind.h"
+#include "base/memory/scoped_refptr.h"
+#include "base/metrics/field_trial.h"
+#include "base/run_loop.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/test/bind_test_util.h"
+#include "base/test/scoped_feature_list.h"
+#include "base/test/scoped_task_environment.h"
+#include "components/signin/public/identity_manager/identity_test_environment.h"
+#include "components/sync_preferences/testing_pref_service_syncable.h"
+#include "components/variations/net/variations_http_headers.h"
+#include "components/variations/variations_associated_data.h"
+#include "components/variations/variations_http_header_provider.h"
+#include "services/network/public/cpp/resource_request.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
+#include "services/network/test/test_url_loader_factory.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+variations::VariationID kVariationID = 123;
+
+void OnDocumentSuggestionsLoaderAvailable(
+ std::unique_ptr<network::SimpleURLLoader> loader) {}
+
+void OnURLLoadComplete(const network::SimpleURLLoader* source,
+ std::unique_ptr<std::string> response_body) {}
+
+class DocumentSuggestionsServiceTest : public testing::Test {
+ protected:
+ DocumentSuggestionsServiceTest()
+ : scoped_task_environment_(
+ base::test::ScopedTaskEnvironment::MainThreadType::UI),
+ shared_url_loader_factory_(
+ base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
+ &test_url_loader_factory_)),
+ identity_test_env_(&test_url_loader_factory_, &prefs_),
+ field_trial_list_(nullptr),
+ document_suggestions_service_(new DocumentSuggestionsService(
+ identity_test_env_.identity_manager(),
+ shared_url_loader_factory_)) {
+ // Set up identity manager.
+ identity_test_env_.SetPrimaryAccount("email");
+ identity_test_env_.SetRefreshTokenForPrimaryAccount();
+ identity_test_env_.SetAutomaticIssueOfAccessTokens(true);
+
+ // Set up a variation.
+ variations::VariationsHttpHeaderProvider::GetInstance()->ResetForTesting();
+ variations::AssociateGoogleVariationID(variations::GOOGLE_WEB_PROPERTIES,
+ "trial name", "group name",
+ kVariationID);
+ base::FieldTrialList::CreateFieldTrial("trial name", "group name")->group();
+ }
+
+ base::test::ScopedTaskEnvironment scoped_task_environment_;
+ network::TestURLLoaderFactory test_url_loader_factory_;
+ scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory_;
+ sync_preferences::TestingPrefServiceSyncable prefs_;
+ signin::IdentityTestEnvironment identity_test_env_;
+ base::FieldTrialList field_trial_list_;
+ std::unique_ptr<DocumentSuggestionsService> document_suggestions_service_;
+
+ DISALLOW_COPY_AND_ASSIGN(DocumentSuggestionsServiceTest);
+};
+
+TEST_F(DocumentSuggestionsServiceTest, VariationHeaders) {
+ test_url_loader_factory_.SetInterceptor(
+ base::BindLambdaForTesting([](const network::ResourceRequest& request) {
+ EXPECT_TRUE(variations::HasVariationsHeader(request));
+ std::string variation =
+ variations::VariationsHttpHeaderProvider::GetInstance()
+ ->GetVariationsString();
+ EXPECT_EQ(variation, " " + base::NumberToString(kVariationID) + " ");
+ }));
+
+ document_suggestions_service_->CreateDocumentSuggestionsRequest(
+ base::ASCIIToUTF16(""), false,
+ base::BindOnce(OnDocumentSuggestionsLoaderAvailable),
+ base::BindOnce(OnURLLoadComplete));
+
+ base::RunLoop().RunUntilIdle();
+}
+
+} // namespace
