Ensure that chrome suggestions are never served if the user doesn't sync his...

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@278111 0039d316-1c4b-4281-b951-d872f2087c98

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;
   };
 

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)
@@ -526,8 +538,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);
   }
 }
 

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,

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, &params);
   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, &params);
+  EXPECT_EQ(test_identity, params.a);
+  ASSERT_FALSE(params.b);
+}
+
 class TabTitleObserver : public content::WebContentsObserver {
  public:
   explicit TabTitleObserver(content::WebContents* contents)