Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
Query the existence other forms of browsing history.
Browse files Browse the repository at this point in the 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
[email protected],[email protected],[email protected],[email protected],[email protected],[email protected],[email protected]

Review-Url: https://codereview.chromium.org/1983073002
Cr-Commit-Position: refs/heads/master@{#395894}
(cherry picked from commit 6d43c19)

Review URL: https://codereview.chromium.org/2087883002 .

Cr-Commit-Position: refs/branch-heads/2743@{#432}
Cr-Branched-From: 2b3ae3b-refs/heads/master@{#394939}
  • Loading branch information
msramek committed Jun 21, 2016
1 parent f6ba157 commit 6d0829c
Show file tree
Hide file tree
Showing 20 changed files with 600 additions and 49 deletions.
3 changes: 3 additions & 0 deletions chrome/browser/android/preferences/pref_service_bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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<jobject>(env, listener))));
}
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ui/webui/options/clear_browser_data_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()));
}
Expand Down
1 change: 1 addition & 0 deletions components/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion components/browsing_data_ui.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@
'type': 'static_library',
'dependencies': [
'../base/base.gyp:base',
'browser_sync_browser',
'history_core_browser',
'sync_driver',
'version_info',
],
'include_dirs': [
'..',
Expand Down
22 changes: 21 additions & 1 deletion components/browsing_data_ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
]
}
8 changes: 7 additions & 1 deletion components/browsing_data_ui/DEPS
Original file line number Diff line number Diff line change
@@ -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",
]
91 changes: 85 additions & 6 deletions components/browsing_data_ui/history_notice_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,67 @@

#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<void(bool)>& 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<void(bool)> target_callback_;
bool final_response_;
int call_count_;
};

} // namespace

namespace browsing_data_ui {

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<void(bool)> callback) {
if (!sync_service ||
!sync_service->IsSyncActive() ||
!sync_service->GetActiveDataTypes().Has(
syncer::HISTORY_DELETE_DIRECTIVES) ||
sync_service->IsUsingSecondaryPassphrase() ||
!history_service) {
callback.Run(false);
Expand All @@ -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<void(bool)> 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<void(bool)> 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;
}
Expand Down
30 changes: 24 additions & 6 deletions components/browsing_data_ui/history_notice_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <string>

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 {
Expand All @@ -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<void(bool)> 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<void(bool)> 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<void(bool)> callback);

Expand Down
Loading

0 comments on commit 6d0829c

Please sign in to comment.