From 6d0829c5b6a5bfa19cf6ca59751750a089e3e476 Mon Sep 17 00:00:00 2001 From: Martin Sramek Date: Tue, 21 Jun 2016 19:35:41 +0200 Subject: [PATCH] Query the existence other forms of browsing history. This CL implements the query for other forms of browsing history, which is necessary to show the notices in the Clear Browsing Data dialog on all platforms. Previously, the query was represented by a dummy method WebHistoryService::HasOtherFormsOfBrowsingHistory(), but now we have an actual endpoint on the Sync server to answer the query. More detailed description of the change: 1. sync/protocol/ Adapt history_status.proto from cl/122116426 and add it to Chromium code. 2. components/history/ Replace the dummy method WebHistoryService::HasOtherFormsOfBrowsingHistory() with the actual request QueryOtherFormsOfBrowsingHistory(). We use most of the existing WebHistoryService insfrastructure, but ReadResponse is replaced by MessageLite::ParseFromString() since we're receiving a protobuf. Furthermore, the query URL is not static, as in other WebHistoryService calls, but has to be generated from the browser user agent and channel. 3. components/browsing_data_ui/ Update the logic to include calls to QueryOtherFormsOfBrowsingHistory(). This call and QueryWebAndAppActivity() call should run in parallel to save time, since they are both HTTP requests. 4. chrome/ Provide the user agent string and the channel info from the Desktop and Android callsites (iOS is in a different codebase) BUG=614652 TBR=dbeam@chromium.org,dfalcantara@chromium.org,cbentzel@chromium.org,pavely@chromium.org,sdefresne@chromium.org,kevinliu@google.com,msarda@chromium.org Review-Url: https://codereview.chromium.org/1983073002 Cr-Commit-Position: refs/heads/master@{#395894} (cherry picked from commit 6d43c1934d9a3a090e16a1346a2ada29b7e1a86c) Review URL: https://codereview.chromium.org/2087883002 . Cr-Commit-Position: refs/branch-heads/2743@{#432} Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939} --- .../preferences/pref_service_bridge.cc | 3 + .../options/clear_browser_data_handler.cc | 2 + components/BUILD.gn | 1 + components/browsing_data_ui.gypi | 3 +- components/browsing_data_ui/BUILD.gn | 22 ++- components/browsing_data_ui/DEPS | 8 +- .../browsing_data_ui/history_notice_utils.cc | 91 ++++++++- .../browsing_data_ui/history_notice_utils.h | 30 ++- .../history_notice_utils_unittest.cc | 187 ++++++++++++++++++ components/components_tests.gyp | 7 +- components/history.gypi | 1 + components/history/core/browser/BUILD.gn | 1 + .../core/browser/web_history_service.cc | 110 +++++++++-- .../core/browser/web_history_service.h | 28 ++- .../browser/web_history_service_unittest.cc | 5 + components/history/core/test/BUILD.gn | 1 + .../core/test/fake_web_history_service.cc | 106 ++++++++-- .../core/test/fake_web_history_service.h | 20 +- sync/protocol/history_status.proto | 22 +++ sync/protocol/protocol.gypi | 1 + 20 files changed, 600 insertions(+), 49 deletions(-) create mode 100644 components/browsing_data_ui/history_notice_utils_unittest.cc create mode 100644 sync/protocol/history_status.proto diff --git a/chrome/browser/android/preferences/pref_service_bridge.cc b/chrome/browser/android/preferences/pref_service_bridge.cc index aa002843bcdcd..7d387cd5ba5e5 100644 --- a/chrome/browser/android/preferences/pref_service_bridge.cc +++ b/chrome/browser/android/preferences/pref_service_bridge.cc @@ -38,9 +38,11 @@ #include "chrome/browser/sync/profile_sync_service_factory.h" #include "chrome/browser/translate/chrome_translate_client.h" #include "chrome/browser/ui/android/android_about_app_info.h" +#include "chrome/common/channel_info.h" #include "chrome/common/chrome_features.h" #include "chrome/common/pref_names.h" #include "chrome/grit/locale_settings.h" +#include "components/browser_sync/browser/profile_sync_service.h" #include "components/browsing_data_ui/history_notice_utils.h" #include "components/content_settings/core/browser/host_content_settings_map.h" #include "components/content_settings/core/common/content_settings.h" @@ -704,6 +706,7 @@ static void RequestInfoAboutOtherFormsOfBrowsingHistory( browsing_data_ui::ShouldPopupDialogAboutOtherFormsOfBrowsingHistory( ProfileSyncServiceFactory::GetForProfile(GetOriginalProfile()), WebHistoryServiceFactory::GetForProfile(GetOriginalProfile()), + chrome::GetChannel(), base::Bind(&EnableDialogAboutOtherFormsOfBrowsingHistory, base::Owned(new ScopedJavaGlobalRef(env, listener)))); } diff --git a/chrome/browser/ui/webui/options/clear_browser_data_handler.cc b/chrome/browser/ui/webui/options/clear_browser_data_handler.cc index ae3433804f91c..339ccb342c0fa 100644 --- a/chrome/browser/ui/webui/options/clear_browser_data_handler.cc +++ b/chrome/browser/ui/webui/options/clear_browser_data_handler.cc @@ -34,6 +34,7 @@ #include "chrome/browser/profiles/profile.h" #include "chrome/browser/sync/profile_sync_service_factory.h" #include "chrome/browser/ui/accelerator_utils.h" +#include "chrome/common/channel_info.h" #include "chrome/common/pref_names.h" #include "chrome/grit/generated_resources.h" #include "chrome/grit/locale_settings.h" @@ -408,6 +409,7 @@ void ClearBrowserDataHandler::RefreshHistoryNotice() { browsing_data_ui::ShouldPopupDialogAboutOtherFormsOfBrowsingHistory( sync_service_, WebHistoryServiceFactory::GetForProfile(Profile::FromWebUI(web_ui())), + chrome::GetChannel(), base::Bind(&ClearBrowserDataHandler::UpdateHistoryDeletionDialog, weak_ptr_factory_.GetWeakPtr())); } diff --git a/components/BUILD.gn b/components/BUILD.gn index 40715d6b80303..ecbbd632cf101 100644 --- a/components/BUILD.gn +++ b/components/BUILD.gn @@ -63,6 +63,7 @@ test("components_unittests") { "//components/bookmarks/browser:unit_tests", "//components/bookmarks/managed:unit_tests", "//components/browser_sync/browser:unit_tests", + "//components/browsing_data_ui:unit_tests", "//components/bubble:unit_tests", "//components/captive_portal:unit_tests", "//components/certificate_reporting:unit_tests", diff --git a/components/browsing_data_ui.gypi b/components/browsing_data_ui.gypi index 859c0cc659986..f7d10e0f6c36f 100644 --- a/components/browsing_data_ui.gypi +++ b/components/browsing_data_ui.gypi @@ -10,8 +10,9 @@ 'type': 'static_library', 'dependencies': [ '../base/base.gyp:base', - 'browser_sync_browser', 'history_core_browser', + 'sync_driver', + 'version_info', ], 'include_dirs': [ '..', diff --git a/components/browsing_data_ui/BUILD.gn b/components/browsing_data_ui/BUILD.gn index 3faf508eb15de..241e32dfefc77 100644 --- a/components/browsing_data_ui/BUILD.gn +++ b/components/browsing_data_ui/BUILD.gn @@ -10,7 +10,27 @@ static_library("browsing_data_ui") { deps = [ "//base", - "//components/browser_sync/browser", "//components/history/core/browser", + "//components/sync_driver:sync_driver", + "//components/version_info", + ] +} + +source_set("unit_tests") { + testonly = true + sources = [ + "history_notice_utils_unittest.cc", + ] + + deps = [ + ":browsing_data_ui", + "//base", + "//components/history/core/test:test", + "//components/signin/core/browser:test_support", + "//components/sync_driver:test_support", + "//components/version_info:version_info", + "//net", + "//sync/protocol:protocol", + "//testing/gtest", ] } diff --git a/components/browsing_data_ui/DEPS b/components/browsing_data_ui/DEPS index d7f9272e71a8b..b798afdcd19e7 100644 --- a/components/browsing_data_ui/DEPS +++ b/components/browsing_data_ui/DEPS @@ -1,4 +1,10 @@ include_rules = [ - "+components/history/core/browser", "+components/browser_sync/browser", + "+components/history/core/browser", + "+components/history/core/test", + "+components/signin", + "+components/sync_driver", + "+components/version_info", + "+sync/internal_api/public", + "+net", ] diff --git a/components/browsing_data_ui/history_notice_utils.cc b/components/browsing_data_ui/history_notice_utils.cc index c1568f2a18837..6a54c26d40745 100644 --- a/components/browsing_data_ui/history_notice_utils.cc +++ b/components/browsing_data_ui/history_notice_utils.cc @@ -4,9 +4,50 @@ #include "components/browsing_data_ui/history_notice_utils.h" +#include "base/bind.h" #include "base/callback.h" -#include "components/browser_sync/browser/profile_sync_service.h" +#include "base/message_loop/message_loop.h" +#include "base/strings/stringprintf.h" #include "components/history/core/browser/web_history_service.h" +#include "components/sync_driver/sync_service.h" +#include "components/version_info/version_info.h" + +namespace { + +// Merges several asynchronous boolean callbacks into one that returns a boolean +// product of their responses. Deletes itself when done. +class MergeBooleanCallbacks { + public: + // Constructor. Upon receiving |expected_call_count| calls to |RunCallback|, + // |target_callback| will be run with the boolean product of their results. + MergeBooleanCallbacks( + int expected_call_count, + const base::Callback& target_callback) + : expected_call_count_(expected_call_count), + target_callback_(target_callback), + final_response_(true), + call_count_(0) {} + + // This method is to be bound to all asynchronous callbacks which we want + // to merge. + void RunCallback(bool response) { + final_response_ &= response; + + if (++call_count_ < expected_call_count_) + return; + + target_callback_.Run(final_response_); + base::MessageLoop::current()->DeleteSoon(FROM_HERE, this); + } + + private: + int expected_call_count_; + base::Callback target_callback_; + bool final_response_; + int call_count_; +}; + +} // namespace namespace browsing_data_ui { @@ -14,14 +55,16 @@ namespace testing { bool g_override_other_forms_of_browsing_history_query = false; -} +} // namespace testing void ShouldShowNoticeAboutOtherFormsOfBrowsingHistory( - const ProfileSyncService* sync_service, + const sync_driver::SyncService* sync_service, history::WebHistoryService* history_service, base::Callback callback) { if (!sync_service || !sync_service->IsSyncActive() || + !sync_service->GetActiveDataTypes().Has( + syncer::HISTORY_DELETE_DIRECTIVES) || sync_service->IsUsingSecondaryPassphrase() || !history_service) { callback.Run(false); @@ -32,12 +75,48 @@ void ShouldShowNoticeAboutOtherFormsOfBrowsingHistory( } void ShouldPopupDialogAboutOtherFormsOfBrowsingHistory( - const ProfileSyncService* sync_service, + const sync_driver::SyncService* sync_service, + history::WebHistoryService* history_service, + version_info::Channel channel, + base::Callback callback) { + // If the query for other forms of browsing history is overriden for testing, + // the conditions are identical with + // ShouldShowNoticeAboutOtherFormsOfBrowsingHistory. + if (testing::g_override_other_forms_of_browsing_history_query) { + ShouldShowNoticeAboutOtherFormsOfBrowsingHistory( + sync_service, history_service, callback); + return; + } + + if (!sync_service || + !sync_service->IsSyncActive() || + !sync_service->GetActiveDataTypes().Has( + syncer::HISTORY_DELETE_DIRECTIVES) || + sync_service->IsUsingSecondaryPassphrase() || + !history_service) { + callback.Run(false); + return; + } + + // Return the boolean product of QueryWebAndAppActivity and + // QueryOtherFormsOfBrowsingHistory. MergeBooleanCallbacks deletes itself + // after processing both callbacks. + MergeBooleanCallbacks* merger = new MergeBooleanCallbacks(2, callback); + history_service->QueryWebAndAppActivity(base::Bind( + &MergeBooleanCallbacks::RunCallback, base::Unretained(merger))); + history_service->QueryOtherFormsOfBrowsingHistory( + channel, + base::Bind( + &MergeBooleanCallbacks::RunCallback, base::Unretained(merger))); +} + +// TODO(crbug.com/614319): This function is deprecated and should be removed. +void ShouldPopupDialogAboutOtherFormsOfBrowsingHistory( + const sync_driver::SyncService* sync_service, history::WebHistoryService* history_service, base::Callback callback) { if (!history_service || - (!testing::g_override_other_forms_of_browsing_history_query && - !history_service->HasOtherFormsOfBrowsingHistory())) { + !testing::g_override_other_forms_of_browsing_history_query) { callback.Run(false); return; } diff --git a/components/browsing_data_ui/history_notice_utils.h b/components/browsing_data_ui/history_notice_utils.h index c3ab520d5ee69..96fabd4947874 100644 --- a/components/browsing_data_ui/history_notice_utils.h +++ b/components/browsing_data_ui/history_notice_utils.h @@ -5,14 +5,22 @@ #ifndef COMPONENTS_BROWSING_DATA_UI_HISTORY_NOTICE_UTILS_H_ #define COMPONENTS_BROWSING_DATA_UI_HISTORY_NOTICE_UTILS_H_ -#include "base/callback_forward.h" +#include -class ProfileSyncService; +#include "base/callback_forward.h" namespace history { class WebHistoryService; } +namespace sync_driver { +class SyncService; +} + +namespace version_info { +enum class Channel; +} + namespace browsing_data_ui { namespace testing { @@ -23,22 +31,32 @@ namespace testing { // found. Used only for testing. The default is false. extern bool g_override_other_forms_of_browsing_history_query; -} +} // testing // Whether the Clear Browsing Data UI should show a notice about the existence // of other forms of browsing history stored in user's account. The response // is returned in a |callback|. void ShouldShowNoticeAboutOtherFormsOfBrowsingHistory( - const ProfileSyncService* sync_service, + const sync_driver::SyncService* sync_service, history::WebHistoryService* history_service, base::Callback callback); // Whether the Clear Browsing Data UI should popup a dialog with information // about the existence of other forms of browsing history stored in user's // account when the user deletes their browsing history for the first time. -// The response is returned in a |callback|. +// The response is returned in a |callback|. The |channel| parameter +// must be provided for successful communication with the Sync server, but +// the result does not depend on it. +void ShouldPopupDialogAboutOtherFormsOfBrowsingHistory( + const sync_driver::SyncService* sync_service, + history::WebHistoryService* history_service, + version_info::Channel channel, + base::Callback callback); + +// A deprecated overloaded version of the above function called by iOS. +// TODO(crbug.com/614319): Remove this when iOS calls the correct version. void ShouldPopupDialogAboutOtherFormsOfBrowsingHistory( - const ProfileSyncService* sync_service, + const sync_driver::SyncService* sync_service, history::WebHistoryService* history_service, base::Callback callback); diff --git a/components/browsing_data_ui/history_notice_utils_unittest.cc b/components/browsing_data_ui/history_notice_utils_unittest.cc new file mode 100644 index 0000000000000..e850b74f2f8d6 --- /dev/null +++ b/components/browsing_data_ui/history_notice_utils_unittest.cc @@ -0,0 +1,187 @@ +// Copyright 2016 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/browsing_data_ui/history_notice_utils.h" + +#include + +#include "base/callback.h" +#include "base/message_loop/message_loop.h" +#include "base/run_loop.h" +#include "components/history/core/test/fake_web_history_service.h" +#include "components/signin/core/browser/account_tracker_service.h" +#include "components/signin/core/browser/fake_profile_oauth2_token_service.h" +#include "components/signin/core/browser/fake_signin_manager.h" +#include "components/signin/core/browser/test_signin_client.h" +#include "components/sync_driver/fake_sync_service.h" +#include "components/version_info/version_info.h" +#include "net/http/http_status_code.h" +#include "net/url_request/url_request_test_util.h" +#include "sync/internal_api/public/base/model_type.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace browsing_data_ui { + +namespace { + +class TestSyncService : public sync_driver::FakeSyncService { + public: + // Getters (FakeSyncService implementation). --------------------------------- + bool IsSyncActive() const override { + return sync_active_; + } + + syncer::ModelTypeSet GetActiveDataTypes() const override { + return active_data_types_; + } + + bool IsUsingSecondaryPassphrase() const override { + return using_secondary_passphrase_; + } + + // Setters. ------------------------------------------------------------------ + void set_sync_active(bool active) { + sync_active_ = active; + } + + void set_active_data_types(syncer::ModelTypeSet data_types) { + active_data_types_ = data_types; + } + + void set_using_secondary_passphrase(bool passphrase) { + using_secondary_passphrase_ = passphrase; + } + + private: + syncer::ModelTypeSet active_data_types_; + bool using_secondary_passphrase_ = false; + bool sync_active_ = false; +}; + +} // namespace + + +class HistoryNoticeUtilsTest : public ::testing::Test { + public: + HistoryNoticeUtilsTest() + : signin_client_(nullptr), + signin_manager_(&signin_client_, &account_tracker_) { + } + + void SetUp() override { + sync_service_.reset(new TestSyncService()); + history_service_.reset(new history::FakeWebHistoryService( + &oauth2_token_service_, + &signin_manager_, + url_request_context_)); + history_service_->SetupFakeResponse(true /* success */, net::HTTP_OK); + } + + TestSyncService* sync_service() { + return sync_service_.get(); + } + + history::FakeWebHistoryService* history_service() { + return history_service_.get(); + } + + void ExpectShouldPopupDialogAboutOtherFormsOfBrowsingHistoryWithResult( + bool expected_test_case_result) { + bool got_result = false; + + ShouldPopupDialogAboutOtherFormsOfBrowsingHistory( + sync_service_.get(), + history_service_.get(), + version_info::Channel::STABLE, + base::Bind( + &HistoryNoticeUtilsTest::Callback, + base::Unretained(this), + base::Unretained(&got_result))); + + if (!got_result) { + run_loop_.reset(new base::RunLoop()); + run_loop_->Run(); + } + + // Process the DeleteSoon() called on MergeBooleanCallbacks, otherwise + // this it will be considered to be leaked. + base::MessageLoop::current()->RunUntilIdle(); + + EXPECT_EQ(expected_test_case_result, result_); + } + + private: + void Callback(bool* got_result, bool result) { + *got_result = true; + result_ = result; + + if (run_loop_) + run_loop_->Quit(); + } + + FakeProfileOAuth2TokenService oauth2_token_service_; + AccountTrackerService account_tracker_; + TestSigninClient signin_client_; + FakeSigninManagerBase signin_manager_; + scoped_refptr url_request_context_; + std::unique_ptr sync_service_; + std::unique_ptr history_service_; + + std::unique_ptr run_loop_; + bool result_; + + base::MessageLoop message_loop_; +}; + +TEST_F(HistoryNoticeUtilsTest, NotSyncing) { + ExpectShouldPopupDialogAboutOtherFormsOfBrowsingHistoryWithResult(false); +} + +TEST_F(HistoryNoticeUtilsTest, SyncingWithWrongParameters) { + sync_service()->set_sync_active(true); + + // Regardless of the state of the web history... + history_service()->SetWebAndAppActivityEnabled(true); + history_service()->SetOtherFormsOfBrowsingHistoryPresent(true); + + // ...the response is false if there's custom passphrase... + sync_service()->set_active_data_types(syncer::ModelTypeSet::All()); + sync_service()->set_using_secondary_passphrase(true); + ExpectShouldPopupDialogAboutOtherFormsOfBrowsingHistoryWithResult(false); + + // ...or even if there's no custom passphrase, but we're not syncing history. + syncer::ModelTypeSet only_passwords(syncer::PASSWORDS); + sync_service()->set_active_data_types(only_passwords); + sync_service()->set_using_secondary_passphrase(false); + ExpectShouldPopupDialogAboutOtherFormsOfBrowsingHistoryWithResult(false); +} + +TEST_F(HistoryNoticeUtilsTest, WebHistoryStates) { + // If history Sync is active... + sync_service()->set_sync_active(true); + sync_service()->set_active_data_types(syncer::ModelTypeSet::All()); + + // ...the result is true if both web history queries return true... + history_service()->SetWebAndAppActivityEnabled(true); + history_service()->SetOtherFormsOfBrowsingHistoryPresent(true); + ExpectShouldPopupDialogAboutOtherFormsOfBrowsingHistoryWithResult(true); + + // ...but not otherwise. + history_service()->SetOtherFormsOfBrowsingHistoryPresent(false); + ExpectShouldPopupDialogAboutOtherFormsOfBrowsingHistoryWithResult(false); + history_service()->SetWebAndAppActivityEnabled(false); + ExpectShouldPopupDialogAboutOtherFormsOfBrowsingHistoryWithResult(false); + history_service()->SetOtherFormsOfBrowsingHistoryPresent(true); + ExpectShouldPopupDialogAboutOtherFormsOfBrowsingHistoryWithResult(false); + + // Invalid responses from the web history are interpreted as false. + history_service()->SetWebAndAppActivityEnabled(true); + history_service()->SetOtherFormsOfBrowsingHistoryPresent(true); + history_service()->SetupFakeResponse(true, net::HTTP_INTERNAL_SERVER_ERROR); + ExpectShouldPopupDialogAboutOtherFormsOfBrowsingHistoryWithResult(false); + history_service()->SetupFakeResponse(false, net::HTTP_OK); + ExpectShouldPopupDialogAboutOtherFormsOfBrowsingHistoryWithResult(false); +} + +} // namespace browsing_data_ui diff --git a/components/components_tests.gyp b/components/components_tests.gyp index eb121d14b71f5..c4abb8752d33b 100644 --- a/components/components_tests.gyp +++ b/components/components_tests.gyp @@ -1,4 +1,4 @@ - # Copyright 2014 The Chromium Authors. All rights reserved. +# Copyright 2014 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. @@ -89,6 +89,9 @@ 'browser_watcher/watcher_metrics_provider_win_unittest.cc', 'browser_watcher/window_hang_monitor_win_unittest.cc', ], + 'browsing_data_ui_unittest_sources': [ + 'browsing_data_ui/history_notice_utils_unittest.cc' + ], 'bubble_unittest_sources': [ 'bubble/bubble_manager_mocks.cc', 'bubble/bubble_manager_mocks.h', @@ -976,6 +979,7 @@ '<@(bookmarks_unittest_sources)', '<@(browser_sync_unittest_sources)', '<@(browser_watcher_unittest_sources)', + '<@(browsing_data_ui_unittest_sources)', '<@(bubble_unittest_sources)', '<@(captive_portal_unittest_sources)', '<@(cast_certificate_unittest_sources)', @@ -1085,6 +1089,7 @@ 'components.gyp:bookmarks_test_support', 'components.gyp:browser_sync_browser', 'components.gyp:browser_sync_browser_test_support', + 'components.gyp:browsing_data_ui', 'components.gyp:bubble', 'components.gyp:captive_portal_test_support', 'components.gyp:cast_certificate', diff --git a/components/history.gypi b/components/history.gypi index 0948c70c36c7b..580eee38fbece 100644 --- a/components/history.gypi +++ b/components/history.gypi @@ -31,6 +31,7 @@ 'signin_core_browser', 'sync_driver', 'url_formatter/url_formatter.gyp:url_formatter', + 'version_info', ], 'export_dependent_settings': [ '../skia/skia.gyp:skia', diff --git a/components/history/core/browser/BUILD.gn b/components/history/core/browser/BUILD.gn index 67f9ee408d534..855753158f2d5 100644 --- a/components/history/core/browser/BUILD.gn +++ b/components/history/core/browser/BUILD.gn @@ -99,6 +99,7 @@ static_library("browser") { "//components/signin/core/browser", "//components/sync_driver", "//components/url_formatter", + "//components/version_info", "//google_apis", "//net", "//sql", diff --git a/components/history/core/browser/web_history_service.cc b/components/history/core/browser/web_history_service.cc index 3a0aa2906f004..2b14601b81dd4 100644 --- a/components/history/core/browser/web_history_service.cc +++ b/components/history/core/browser/web_history_service.cc @@ -4,15 +4,21 @@ #include "components/history/core/browser/web_history_service.h" +#include + #include "base/bind.h" +#include "base/command_line.h" #include "base/json/json_reader.h" #include "base/json/json_writer.h" #include "base/metrics/histogram.h" +#include "base/optional.h" #include "base/stl_util.h" #include "base/strings/string_number_conversions.h" #include "base/strings/utf_string_conversions.h" #include "base/values.h" #include "components/signin/core/browser/signin_manager.h" +#include "components/sync_driver/local_device_info_provider_impl.h" +#include "components/sync_driver/sync_util.h" #include "google_apis/gaia/gaia_urls.h" #include "google_apis/gaia/google_service_auth_error.h" #include "google_apis/gaia/oauth2_token_service.h" @@ -23,6 +29,8 @@ #include "net/url_request/url_fetcher.h" #include "net/url_request/url_fetcher_delegate.h" #include "net/url_request/url_request_context_getter.h" +#include "sync/protocol/history_status.pb.h" +#include "ui/base/device_form_factor.h" #include "url/gurl.h" namespace history { @@ -47,8 +55,12 @@ const char kHistoryAudioHistoryChangeUrl[] = const char kQueryWebAndAppActivityUrl[] = "https://history.google.com/history/api/lookup?client=web_app"; +const char kQueryOtherFormsOfBrowsingHistoryUrlSuffix[] = "/historystatus"; + const char kPostDataMimeType[] = "text/plain"; +const char kSyncProtoMimeType[] = "application/octet-stream"; + // The maximum number of retries for the URLFetcher requests. const size_t kMaxRetries = 1; @@ -81,6 +93,7 @@ class RequestImpl : public WebHistoryService::Request, signin_manager_(signin_manager), request_context_(request_context), url_(url), + post_data_mime_type_(kPostDataMimeType), response_code_(0), auth_retry_count_(0), callback_(callback), @@ -160,8 +173,8 @@ class RequestImpl : public WebHistoryService::Request, // Helper for creating a new URLFetcher for the API request. std::unique_ptr CreateUrlFetcher( const std::string& access_token) { - net::URLFetcher::RequestType request_type = post_data_.empty() ? - net::URLFetcher::GET : net::URLFetcher::POST; + net::URLFetcher::RequestType request_type = post_data_ ? + net::URLFetcher::POST : net::URLFetcher::GET; std::unique_ptr fetcher = net::URLFetcher::Create(url_, request_type, this); fetcher->SetRequestContext(request_context_.get()); @@ -171,13 +184,30 @@ class RequestImpl : public WebHistoryService::Request, fetcher->AddExtraRequestHeader("Authorization: Bearer " + access_token); fetcher->AddExtraRequestHeader("X-Developer-Key: " + GaiaUrls::GetInstance()->oauth2_chrome_client_id()); - if (request_type == net::URLFetcher::POST) - fetcher->SetUploadData(kPostDataMimeType, post_data_); + + if (!user_agent_.empty()) { + fetcher->AddExtraRequestHeader( + std::string(net::HttpRequestHeaders::kUserAgent) + + ": " + user_agent_); + } + + if (post_data_) + fetcher->SetUploadData(post_data_mime_type_, post_data_.value()); return fetcher; } void SetPostData(const std::string& post_data) override { + SetPostDataAndType(post_data, kPostDataMimeType); + } + + void SetPostDataAndType(const std::string& post_data, + const std::string& mime_type) override { post_data_ = post_data; + post_data_mime_type_ = mime_type; + } + + void SetUserAgent(const std::string& user_agent) override { + user_agent_ = user_agent; } OAuth2TokenService* token_service_; @@ -188,7 +218,13 @@ class RequestImpl : public WebHistoryService::Request, GURL url_; // POST data to be sent with the request (may be empty). - std::string post_data_; + base::Optional post_data_; + + // MIME type of the post requests. Defaults to text/plain. + std::string post_data_mime_type_; + + // The user agent header used with this request. + std::string user_agent_; // The OAuth2 access token request. std::unique_ptr token_request_; @@ -302,6 +338,7 @@ WebHistoryService::~WebHistoryService() { STLDeleteElements(&pending_expire_requests_); STLDeleteElements(&pending_audio_history_requests_); STLDeleteElements(&pending_web_and_app_activity_requests_); + STLDeleteElements(&pending_other_forms_of_browsing_history_requests_); } WebHistoryService::Request* WebHistoryService::CreateRequest( @@ -441,12 +478,6 @@ size_t WebHistoryService::GetNumberOfPendingAudioHistoryRequests() { return pending_audio_history_requests_.size(); } -bool WebHistoryService::HasOtherFormsOfBrowsingHistory() const { - // TODO(msramek): Query history.google.com for existence of other forms of - // browsing history. In the meantime, assume that there isn't. - return false; -} - void WebHistoryService::QueryWebAndAppActivity( const QueryWebAndAppActivityCallback& callback) { // Wrap the original callback into a generic completion callback. @@ -461,6 +492,46 @@ void WebHistoryService::QueryWebAndAppActivity( request->Start(); } +void WebHistoryService::QueryOtherFormsOfBrowsingHistory( + version_info::Channel channel, + const QueryOtherFormsOfBrowsingHistoryCallback& callback) { + // Wrap the original callback into a generic completion callback. + CompletionCallback completion_callback = base::Bind( + &WebHistoryService::QueryOtherFormsOfBrowsingHistoryCompletionCallback, + weak_ptr_factory_.GetWeakPtr(), + callback); + + // Find the Sync request URL. + GURL url = + GetSyncServiceURL(*base::CommandLine::ForCurrentProcess(), channel); + GURL::Replacements replace_path; + std::string new_path = + url.path() + kQueryOtherFormsOfBrowsingHistoryUrlSuffix; + replace_path.SetPathStr(new_path); + url = url.ReplaceComponents(replace_path); + DCHECK(url.is_valid()); + + Request* request = CreateRequest(url, completion_callback); + + // Set the Sync-specific user agent. + // TODO(pavely): Refactor LocalDeviceInfoProviderImpl::GetSyncUserAgent() + // to a standalone function. + browser_sync::LocalDeviceInfoProviderImpl local_device_info_provider_( + channel, std::string() /* version (unused) */, + ui::GetDeviceFormFactor() == ui::DEVICE_FORM_FACTOR_TABLET); + request->SetUserAgent(local_device_info_provider_.GetSyncUserAgent()); + + pending_other_forms_of_browsing_history_requests_.insert(request); + + // Set the request protobuf. + sync_pb::HistoryStatusRequest request_proto; + std::string post_data; + request_proto.SerializeToString(&post_data); + request->SetPostDataAndType(post_data, kSyncProtoMimeType); + + request->Start(); +} + // static void WebHistoryService::QueryHistoryCompletionCallback( const WebHistoryService::QueryWebHistoryCallback& callback, @@ -530,4 +601,21 @@ void WebHistoryService::QueryWebAndAppActivityCompletionCallback( callback.Run(web_and_app_activity_enabled); } +void WebHistoryService::QueryOtherFormsOfBrowsingHistoryCompletionCallback( + const WebHistoryService::QueryOtherFormsOfBrowsingHistoryCallback& callback, + WebHistoryService::Request* request, + bool success) { + pending_other_forms_of_browsing_history_requests_.erase(request); + std::unique_ptr request_ptr(request); + + bool has_other_forms_of_browsing_history = false; + if (success && request->GetResponseCode() == net::HTTP_OK) { + sync_pb::HistoryStatusResponse history_status; + if (history_status.ParseFromString(request->GetResponseBody())) + has_other_forms_of_browsing_history = history_status.has_derived_data(); + } + + callback.Run(has_other_forms_of_browsing_history); +} + } // namespace history diff --git a/components/history/core/browser/web_history_service.h b/components/history/core/browser/web_history_service.h index 2b170cf519463..0b6c08bdc8683 100644 --- a/components/history/core/browser/web_history_service.h +++ b/components/history/core/browser/web_history_service.h @@ -25,6 +25,10 @@ namespace net { class URLRequestContextGetter; } +namespace version_info { +enum class Channel; +} + class OAuth2TokenService; class SigninManagerBase; @@ -55,6 +59,11 @@ class WebHistoryService : public KeyedService { virtual void SetPostData(const std::string& post_data) = 0; + virtual void SetPostDataAndType(const std::string& post_data, + const std::string& mime_type) = 0; + + virtual void SetUserAgent(const std::string& user_agent) = 0; + // Tells the request to begin. virtual void Start() = 0; @@ -75,6 +84,9 @@ class WebHistoryService : public KeyedService { typedef base::Callback QueryWebAndAppActivityCallback; + typedef base::Callback + QueryOtherFormsOfBrowsingHistoryCallback; + typedef base::Callback CompletionCallback; WebHistoryService( @@ -121,7 +133,9 @@ class WebHistoryService : public KeyedService { size_t GetNumberOfPendingAudioHistoryRequests(); // Whether there are other forms of browsing history stored on the server. - bool HasOtherFormsOfBrowsingHistory() const; + void QueryOtherFormsOfBrowsingHistory( + version_info::Channel channel, + const QueryOtherFormsOfBrowsingHistoryCallback& callback); protected: // This function is pulled out for testing purposes. Caller takes ownership of @@ -166,6 +180,14 @@ class WebHistoryService : public KeyedService { WebHistoryService::Request* request, bool success); + // Called by |request| when a query for other forms of browsing history has + // completed. Unpacks the response and calls |callback|, which is the original + // callback that was passed to QueryOtherFormsOfBrowsingHistory(). + void QueryOtherFormsOfBrowsingHistoryCompletionCallback( + const WebHistoryService::QueryWebAndAppActivityCallback& callback, + WebHistoryService::Request* request, + bool success); + private: friend class WebHistoryServiceTest; @@ -193,6 +215,10 @@ class WebHistoryService : public KeyedService { // profile shutdown. std::set pending_web_and_app_activity_requests_; + // Pending queries for other forms of browsing history to be canceled if not + // complete by profile shutdown. + std::set pending_other_forms_of_browsing_history_requests_; + base::WeakPtrFactory weak_ptr_factory_; DISALLOW_COPY_AND_ASSIGN(WebHistoryService); diff --git a/components/history/core/browser/web_history_service_unittest.cc b/components/history/core/browser/web_history_service_unittest.cc index f5e789b28295c..6a62a10c83a63 100644 --- a/components/history/core/browser/web_history_service_unittest.cc +++ b/components/history/core/browser/web_history_service_unittest.cc @@ -122,6 +122,11 @@ class TestRequest : public WebHistoryService::Request { void SetPostData(const std::string& post_data) override { post_data_ = post_data; } + void SetPostDataAndType(const std::string& post_data, + const std::string& mime_type) override { + SetPostData(post_data); + } + void SetUserAgent(const std::string& post_data) override {} void Start() override { is_pending_ = true; diff --git a/components/history/core/test/BUILD.gn b/components/history/core/test/BUILD.gn index cd3d94f92a4d8..9bc56b1fd5a2b 100644 --- a/components/history/core/test/BUILD.gn +++ b/components/history/core/test/BUILD.gn @@ -33,6 +33,7 @@ static_library("test") { "//net", "//sql", "//sql:test_support", + "//sync/protocol:protocol", "//testing/gtest", "//ui/gfx", "//url", diff --git a/components/history/core/test/fake_web_history_service.cc b/components/history/core/test/fake_web_history_service.cc index 2739974eec566..44e5be420c94f 100644 --- a/components/history/core/test/fake_web_history_service.cc +++ b/components/history/core/test/fake_web_history_service.cc @@ -9,9 +9,11 @@ #include "base/callback.h" #include "base/macros.h" #include "base/strings/string_number_conversions.h" +#include "base/strings/stringprintf.h" #include "base/time/time.h" #include "net/base/url_util.h" #include "net/url_request/url_request_context_getter.h" +#include "sync/protocol/history_status.pb.h" namespace history { @@ -19,9 +21,22 @@ namespace history { namespace { +// TODO(msramek): Find a way to keep these URLs in sync with what is used +// in WebHistoryService. + +const char kLookupUrl[] = + "https://history.google.com/history/api/lookup"; + +const char kChromeClient[] = "chrome"; + +const char kWebAndAppClient[] = "web_app"; + +const char kSyncServerHost[] = "clients4.google.com"; + class FakeRequest : public WebHistoryService::Request { public: FakeRequest(FakeWebHistoryService* service, + const GURL& url, bool emulate_success, int emulate_response_code, const WebHistoryService::CompletionCallback& callback, @@ -34,10 +49,14 @@ class FakeRequest : public WebHistoryService::Request { int GetResponseCode() override; const std::string& GetResponseBody() override; void SetPostData(const std::string& post_data) override; + void SetPostDataAndType(const std::string& post_data, + const std::string& mime_type) override; + void SetUserAgent(const std::string& user_agent) override; void Start() override; private: FakeWebHistoryService* service_; + GURL url_; bool emulate_success_; int emulate_response_code_; const WebHistoryService::CompletionCallback& callback_; @@ -52,6 +71,7 @@ class FakeRequest : public WebHistoryService::Request { FakeRequest::FakeRequest( FakeWebHistoryService* service, + const GURL& url, bool emulate_success, int emulate_response_code, const WebHistoryService::CompletionCallback& callback, @@ -59,13 +79,14 @@ FakeRequest::FakeRequest( base::Time end, int max_count) : service_(service), - emulate_success_(emulate_success), - emulate_response_code_(emulate_response_code), - callback_(callback), - begin_(begin), - end_(end), - max_count_(max_count), - is_pending_(false) { + url_(url), + emulate_success_(emulate_success), + emulate_response_code_(emulate_response_code), + callback_(callback), + begin_(begin), + end_(end), + max_count_(max_count), + is_pending_(false) { } bool FakeRequest::IsPending() { @@ -77,14 +98,41 @@ int FakeRequest::GetResponseCode() { } const std::string& FakeRequest::GetResponseBody() { - int count = service_->GetNumberOfVisitsBetween(begin_, end_); - if (max_count_ && max_count_ < count) - count = max_count_; - - response_body_ = "{ \"event\": ["; - for (int i = 0; i < count; ++i) - response_body_ += i ? ", {}" : "{}"; - response_body_ += "] }"; + std::string client; + net::GetValueForKeyInQuery(url_, "client", &client); + + GURL::Replacements remove_query; + remove_query.ClearQuery(); + GURL base_url = url_.ReplaceComponents(remove_query); + + // History query. + if (base_url == GURL(kLookupUrl) && client == kChromeClient) { + int count = service_->GetNumberOfVisitsBetween(begin_, end_); + if (max_count_ && max_count_ < count) + count = max_count_; + + response_body_ = "{ \"event\": ["; + for (int i = 0; i < count; ++i) + response_body_ += i ? ", {}" : "{}"; + response_body_ += "] }"; + } + + // Web and app activity query. + if (base_url == GURL(kLookupUrl) && client == kWebAndAppClient) { + response_body_ = base::StringPrintf( + "{ \"history_recording_enabled\": %s }", + service_->IsWebAndAppActivityEnabled() ? "true" : "false"); + } + + // Other forms of browsing history query. + if (url_.host() == kSyncServerHost) { + std::unique_ptr history_status( + new sync_pb::HistoryStatusResponse()); + history_status->set_has_derived_data( + service_->AreOtherFormsOfBrowsingHistoryPresent()); + history_status->SerializeToString(&response_body_); + } + return response_body_; } @@ -92,6 +140,15 @@ void FakeRequest::SetPostData(const std::string& post_data) { // Unused. }; +void FakeRequest::SetPostDataAndType(const std::string& post_data, + const std::string& mime_type) { + // Unused. +}; + +void FakeRequest::SetUserAgent(const std::string& user_agent) { + // Unused. +}; + void FakeRequest::Start() { is_pending_ = true; callback_.Run(this, emulate_success_); @@ -165,8 +222,25 @@ FakeWebHistoryService::Request* FakeWebHistoryService::CreateRequest( if (net::GetValueForKeyInQuery(url, "num", &max_count_str)) base::StringToInt(max_count_str, &max_count); - return new FakeRequest(this, emulate_success_, emulate_response_code_, + return new FakeRequest(this, url, emulate_success_, emulate_response_code_, callback, begin, end, max_count); } +bool FakeWebHistoryService::IsWebAndAppActivityEnabled() { + return web_and_app_activity_enabled_; +} + +void FakeWebHistoryService::SetWebAndAppActivityEnabled(bool enabled) { + web_and_app_activity_enabled_ = enabled; +} + +bool FakeWebHistoryService::AreOtherFormsOfBrowsingHistoryPresent() { + return other_forms_of_browsing_history_present_; +} + +void FakeWebHistoryService::SetOtherFormsOfBrowsingHistoryPresent( + bool present) { + other_forms_of_browsing_history_present_ = present; +} + } // namespace history diff --git a/components/history/core/test/fake_web_history_service.h b/components/history/core/test/fake_web_history_service.h index 79a5b37bc5fde..7832e273441ef 100644 --- a/components/history/core/test/fake_web_history_service.h +++ b/components/history/core/test/fake_web_history_service.h @@ -24,11 +24,9 @@ namespace history { // Use |SetupFakeResponse| to influence whether the requests should succeed // or fail, and with which error code. // -// Note: Currently, the only test that uses this class only counts the number -// of visits and does not inspect their contents. Therefore, the behavior -// of this class is only defined for |WebHistoryService::QueryHistory| queries -// and even for them, we only return the correct number of items, without -// contents. +// Note: The behavior of this class is only defined for some WebHistoryService +// queries. If needed, improve FakeRequest::GetResponseBody() to simulate +// responses for other queries. // // TODO(msramek): This class might need its own set of tests. class FakeWebHistoryService : public history::WebHistoryService { @@ -53,6 +51,14 @@ class FakeWebHistoryService : public history::WebHistoryService { // Counts the number of visits within a certain time range. int GetNumberOfVisitsBetween(const base::Time& begin, const base::Time& end); + // Get and set the fake state of web and app activity. + bool IsWebAndAppActivityEnabled(); + void SetWebAndAppActivityEnabled(bool enabled); + + // Get and set the fake state of other forms of browsing history. + bool AreOtherFormsOfBrowsingHistoryPresent(); + void SetOtherFormsOfBrowsingHistoryPresent(bool present); + private: base::Time GetTimeForKeyInQuery(const GURL& url, const std::string& key); @@ -64,6 +70,10 @@ class FakeWebHistoryService : public history::WebHistoryService { bool emulate_success_; int emulate_response_code_; + // States of serverside corpora. + bool web_and_app_activity_enabled_; + bool other_forms_of_browsing_history_present_; + // Fake visits storage. typedef std::pair Visit; std::vector visits_; diff --git a/sync/protocol/history_status.proto b/sync/protocol/history_status.proto new file mode 100644 index 0000000000000..e0f5ac9eaf27f --- /dev/null +++ b/sync/protocol/history_status.proto @@ -0,0 +1,22 @@ +// Copyright 2016 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 + +syntax = "proto2"; + +option optimize_for = LITE_RUNTIME; +option retain_unknown_fields = true; + +package sync_pb; + +message HistoryStatusRequest { +} + +// Response to a history status request. +message HistoryStatusResponse { + // Minimal time to wait before issuing another request. + optional int32 min_poll_interval_seconds = 1 [default = 3600]; + + // Indicates whether the history corpuses have any derived data for a user. + optional bool has_derived_data = 2; +} diff --git a/sync/protocol/protocol.gypi b/sync/protocol/protocol.gypi index 183b7db0009ea..f522c5f8b1706 100644 --- a/sync/protocol/protocol.gypi +++ b/sync/protocol/protocol.gypi @@ -34,6 +34,7 @@ '<(sync_proto_sources_dir)/favicon_tracking_specifics.proto', '<(sync_proto_sources_dir)/get_updates_caller_info.proto', '<(sync_proto_sources_dir)/history_delete_directive_specifics.proto', + '<(sync_proto_sources_dir)/history_status.proto', '<(sync_proto_sources_dir)/nigori_specifics.proto', '<(sync_proto_sources_dir)/managed_user_setting_specifics.proto', '<(sync_proto_sources_dir)/managed_user_shared_setting_specifics.proto',