diff options
author | beaudoin@chromium.org <beaudoin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-06-20 17:25:11 +0000 |
---|---|---|
committer | beaudoin@chromium.org <beaudoin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-06-20 17:25:11 +0000 |
commit | f352bc8314586155f3f30cb79e1d5f3368182309 (patch) | |
tree | 6696101c571abf637ee07d49c395cb431f1c5017 | |
parent | 5a8ac14e1843ac9ae81673277dcad4e66d800045 (diff) | |
download | chromium_src-f352bc8314586155f3f30cb79e1d5f3368182309.zip chromium_src-f352bc8314586155f3f30cb79e1d5f3368182309.tar.gz chromium_src-f352bc8314586155f3f30cb79e1d5f3368182309.tar.bz2 |
Merge 278111 "Ensure that chrome suggestions are never served if..."
> Ensure that chrome suggestions are never served if the user doesn't sync his history on a given machine, even if he syncs it on other machines.
>
> BUG=384913
>
> Review URL: https://codereview.chromium.org/336413006
TBR=beaudoin@chromium.org
Review URL: https://codereview.chromium.org/345993002
git-svn-id: svn://svn.chromium.org/chrome/branches/1985/src@278743 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/ui/search/search_ipc_router.h | 4 | ||||
-rw-r--r-- | chrome/browser/ui/search/search_tab_helper.cc | 19 | ||||
-rw-r--r-- | chrome/browser/ui/search/search_tab_helper.h | 2 | ||||
-rw-r--r-- | chrome/browser/ui/search/search_tab_helper_unittest.cc | 54 |
4 files changed, 73 insertions, 6 deletions
diff --git a/chrome/browser/ui/search/search_ipc_router.h b/chrome/browser/ui/search/search_ipc_router.h index 22db172..140d1c1 100644 --- a/chrome/browser/ui/search/search_ipc_router.h +++ b/chrome/browser/ui/search/search_ipc_router.h @@ -77,6 +77,10 @@ class SearchIPCRouter : public content::WebContentsObserver { // Called when the SearchBox wants to verify the signed-in Chrome identity // against the provided |identity|. Will make a round-trip to the browser // and eventually return the result through SendChromeIdentityCheckResult. + // Calls SendChromeIdentityCheckResult with true if both the identity + // matches and the user syncs their history. + // TODO(beaudoin): Change this function name and related APIs now that it's + // checking both the identity and the user's sync state. virtual void OnChromeIdentityCheck(const base::string16& identity) = 0; }; diff --git a/chrome/browser/ui/search/search_tab_helper.cc b/chrome/browser/ui/search/search_tab_helper.cc index 4bc6991..5bcba7b 100644 --- a/chrome/browser/ui/search/search_tab_helper.cc +++ b/chrome/browser/ui/search/search_tab_helper.cc @@ -17,6 +17,8 @@ #include "chrome/browser/search/instant_service_factory.h" #include "chrome/browser/search/search.h" #include "chrome/browser/signin/signin_manager_factory.h" +#include "chrome/browser/sync/profile_sync_service.h" +#include "chrome/browser/sync/profile_sync_service_factory.h" #include "chrome/browser/ui/app_list/app_list_util.h" #include "chrome/browser/ui/browser_navigator.h" #include "chrome/browser/ui/browser_window.h" @@ -120,6 +122,16 @@ void RecordNewTabLoadTime(content::WebContents* contents) { core_tab_helper->set_new_tab_start_time(base::TimeTicks()); } +// Returns true if the user is signed in and full history sync is enabled, +// and false otherwise. +bool IsHistorySyncEnabled(Profile* profile) { + ProfileSyncService* sync = + ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile); + return sync && + sync->sync_initialized() && + sync->GetActiveDataTypes().Has(syncer::HISTORY_DELETE_DIRECTIVES); +} + } // namespace SearchTabHelper::SearchTabHelper(content::WebContents* web_contents) @@ -523,8 +535,11 @@ void SearchTabHelper::OnChromeIdentityCheck(const base::string16& identity) { if (manager) { const base::string16 username = base::UTF8ToUTF16(manager->GetAuthenticatedUsername()); - ipc_router_.SendChromeIdentityCheckResult(identity, - identity == username); + // The identity check only passes if the user is syncing their history. + // TODO(beaudoin): Change this function name and related APIs now that it's + // checking both the identity and the user's sync state. + bool matches = IsHistorySyncEnabled(profile()) && identity == username; + ipc_router_.SendChromeIdentityCheckResult(identity, matches); } } diff --git a/chrome/browser/ui/search/search_tab_helper.h b/chrome/browser/ui/search/search_tab_helper.h index 38e6bd0..78f12fb 100644 --- a/chrome/browser/ui/search/search_tab_helper.h +++ b/chrome/browser/ui/search/search_tab_helper.h @@ -115,6 +115,8 @@ class SearchTabHelper : public content::WebContentsObserver, OnChromeIdentityCheckSignedOutMatch); FRIEND_TEST_ALL_PREFIXES(SearchTabHelperTest, OnChromeIdentityCheckSignedOutMismatch); + FRIEND_TEST_ALL_PREFIXES(SearchTabHelperTest, + OnChromeIdentityCheckMatchNotSyncing); FRIEND_TEST_ALL_PREFIXES(SearchTabHelperWindowTest, OnProvisionalLoadFailRedirectNTPToLocal); FRIEND_TEST_ALL_PREFIXES(SearchTabHelperWindowTest, diff --git a/chrome/browser/ui/search/search_tab_helper_unittest.cc b/chrome/browser/ui/search/search_tab_helper_unittest.cc index c554a96..b997fe5 100644 --- a/chrome/browser/ui/search/search_tab_helper_unittest.cc +++ b/chrome/browser/ui/search/search_tab_helper_unittest.cc @@ -12,6 +12,9 @@ #include "chrome/browser/search_engines/template_url_service_factory.h" #include "chrome/browser/signin/fake_signin_manager.h" #include "chrome/browser/signin/signin_manager_factory.h" +#include "chrome/browser/sync/profile_sync_service.h" +#include "chrome/browser/sync/profile_sync_service_factory.h" +#include "chrome/browser/sync/profile_sync_service_mock.h" #include "chrome/browser/ui/search/search_ipc_router.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/common/chrome_switches.h" @@ -36,6 +39,8 @@ #include "ui/base/l10n/l10n_util.h" #include "url/gurl.h" +using testing::Return; + namespace { class MockSearchIPCRouterDelegate : public SearchIPCRouter::Delegate { @@ -71,12 +76,16 @@ class SearchTabHelperTest : public ChromeRenderViewHostTestHarness { TestingProfile::Builder builder; builder.AddTestingFactory(SigninManagerFactory::GetInstance(), FakeSigninManagerBase::Build); + builder.AddTestingFactory( + ProfileSyncServiceFactory::GetInstance(), + ProfileSyncServiceMock::BuildMockProfileSyncService); return builder.Build().release(); } // Creates a sign-in manager for tests. If |username| is not empty, the // testing profile of the WebContents will be connected to the given account. - void CreateSigninManager(const std::string& username) { + // The account can be configured to |sync_history| or not. + void CreateSigninManager(const std::string& username, bool sync_history) { SigninManagerBase* signin_manager = static_cast<SigninManagerBase*>( SigninManagerFactory::GetForProfile(profile())); @@ -84,6 +93,17 @@ class SearchTabHelperTest : public ChromeRenderViewHostTestHarness { ASSERT_TRUE(signin_manager); signin_manager->SetAuthenticatedUsername(username); } + + ProfileSyncServiceMock* sync_service = static_cast<ProfileSyncServiceMock*>( + ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile())); + + EXPECT_CALL(*sync_service, sync_initialized()).WillRepeatedly(Return(true)); + syncer::ModelTypeSet result; + if (sync_history) { + result.Put(syncer::HISTORY_DELETE_DIRECTIVES); + } + EXPECT_CALL(*sync_service, GetActiveDataTypes()) + .WillRepeatedly(Return(result)); } bool MessageWasSent(uint32 id) { @@ -146,7 +166,7 @@ TEST_F(SearchTabHelperTest, PageURLDoesntBelongToInstantRenderer) { TEST_F(SearchTabHelperTest, OnChromeIdentityCheckMatch) { NavigateAndCommit(GURL(chrome::kChromeSearchLocalNtpUrl)); - CreateSigninManager(std::string("foo@bar.com")); + CreateSigninManager(std::string("foo@bar.com"), true); SearchTabHelper* search_tab_helper = SearchTabHelper::FromWebContents(web_contents()); ASSERT_NE(static_cast<SearchTabHelper*>(NULL), search_tab_helper); @@ -166,7 +186,7 @@ TEST_F(SearchTabHelperTest, OnChromeIdentityCheckMatch) { TEST_F(SearchTabHelperTest, OnChromeIdentityCheckMismatch) { NavigateAndCommit(GURL(chrome::kChromeSearchLocalNtpUrl)); - CreateSigninManager(std::string("foo@bar.com")); + CreateSigninManager(std::string("foo@bar.com"), true); SearchTabHelper* search_tab_helper = SearchTabHelper::FromWebContents(web_contents()); ASSERT_NE(static_cast<SearchTabHelper*>(NULL), search_tab_helper); @@ -187,6 +207,9 @@ TEST_F(SearchTabHelperTest, OnChromeIdentityCheckMismatch) { TEST_F(SearchTabHelperTest, OnChromeIdentityCheckSignedOutMatch) { NavigateAndCommit(GURL(chrome::kChromeSearchLocalNtpUrl)); // This test does not sign in. + ProfileSyncServiceMock* sync_service = static_cast<ProfileSyncServiceMock*>( + ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile())); + EXPECT_CALL(*sync_service, sync_initialized()).WillRepeatedly(Return(false)); SearchTabHelper* search_tab_helper = SearchTabHelper::FromWebContents(web_contents()); ASSERT_NE(static_cast<SearchTabHelper*>(NULL), search_tab_helper); @@ -201,12 +224,15 @@ TEST_F(SearchTabHelperTest, OnChromeIdentityCheckSignedOutMatch) { ChromeViewMsg_ChromeIdentityCheckResult::Param params; ChromeViewMsg_ChromeIdentityCheckResult::Read(message, ¶ms); EXPECT_EQ(test_identity, params.a); - ASSERT_TRUE(params.b); + ASSERT_FALSE(params.b); } TEST_F(SearchTabHelperTest, OnChromeIdentityCheckSignedOutMismatch) { NavigateAndCommit(GURL(chrome::kChromeSearchLocalNtpUrl)); // This test does not sign in. + ProfileSyncServiceMock* sync_service = static_cast<ProfileSyncServiceMock*>( + ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile())); + EXPECT_CALL(*sync_service, sync_initialized()).WillRepeatedly(Return(false)); SearchTabHelper* search_tab_helper = SearchTabHelper::FromWebContents(web_contents()); ASSERT_NE(static_cast<SearchTabHelper*>(NULL), search_tab_helper); @@ -224,6 +250,26 @@ TEST_F(SearchTabHelperTest, OnChromeIdentityCheckSignedOutMismatch) { ASSERT_FALSE(params.b); } +TEST_F(SearchTabHelperTest, OnChromeIdentityCheckMatchNotSyncing) { + NavigateAndCommit(GURL(chrome::kChromeSearchLocalNtpUrl)); + CreateSigninManager(std::string("foo@bar.com"), false); + SearchTabHelper* search_tab_helper = + SearchTabHelper::FromWebContents(web_contents()); + ASSERT_NE(static_cast<SearchTabHelper*>(NULL), search_tab_helper); + + const base::string16 test_identity = base::ASCIIToUTF16("foo@bar.com"); + search_tab_helper->OnChromeIdentityCheck(test_identity); + + const IPC::Message* message = process()->sink().GetUniqueMessageMatching( + ChromeViewMsg_ChromeIdentityCheckResult::ID); + ASSERT_TRUE(message != NULL); + + ChromeViewMsg_ChromeIdentityCheckResult::Param params; + ChromeViewMsg_ChromeIdentityCheckResult::Read(message, ¶ms); + EXPECT_EQ(test_identity, params.a); + ASSERT_FALSE(params.b); +} + class TabTitleObserver : public content::WebContentsObserver { public: explicit TabTitleObserver(content::WebContents* contents) |