SearchTabHelper: Add IsSyncFeatureEnabled check to IsHistorySyncEnabled

Previously it just checked GetPreferredDataTypes. This was probably
okay in practice (the check was anyway more of a "has the user not
disabled history sync" rather than "is history sync enabled"), but
still better to be correct.

While we're here, also move to SyncUserSettings, which is the new way
to query any user-configurable bits of Sync.

This also includes a bunch of test cleanup: Removing a Mock class that
isn't used, and using NiceMock to avoid unnecessary chattiness.

Bug: 907027, 884159
Change-Id: I2a3d0f8ef13a74b51f4b4a2edc19976a61c6c430
Reviewed-on: https://chromium-review.googlesource.com/c/1343087Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610055}

chrome/browser/ui/search/search_tab_helper.cc

@@ -96,9 +96,9 @@ void RecordNewTabLoadTime(content::WebContents* contents) {
 // their history.
 bool IsHistorySyncEnabled(Profile* profile) {
   browser_sync::ProfileSyncService* sync =
-      ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile);
-  return sync &&
-      sync->GetPreferredDataTypes().Has(syncer::HISTORY_DELETE_DIRECTIVES);
+      ProfileSyncServiceFactory::GetForProfile(profile);
+  return sync->IsSyncFeatureEnabled() &&
+         sync->GetUserSettings()->GetChosenDataTypes().Has(syncer::TYPED_URLS);
 }
 
 }  // namespace

chrome/browser/ui/search/search_tab_helper_unittest.cc

@@ -8,7 +8,6 @@
 
 #include <memory>
 #include <string>
-#include <tuple>
 #include <utility>
 
 #include "base/bind.h"
@@ -18,8 +17,6 @@
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/sync/profile_sync_test_util.h"
 #include "chrome/browser/ui/search/search_ipc_router.h"
-#include "chrome/browser/ui/tabs/tab_strip_model.h"
-#include "chrome/common/render_messages.h"
 #include "chrome/common/search/mock_embedded_search_client.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/generated_resources.h"
@@ -27,64 +24,19 @@
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/browser_sync/profile_sync_service.h"
-#include "components/omnibox/common/omnibox_focus_state.h"
 #include "components/strings/grit/components_strings.h"
-#include "content/public/browser/navigation_controller.h"
-#include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/web_contents.h"
-#include "content/public/test/mock_render_process_host.h"
-#include "ipc/ipc_message.h"
-#include "ipc/ipc_test_sink.h"
-#include "net/base/net_errors.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "url/gurl.h"
 
-class OmniboxView;
-
-using testing::Eq;
-using testing::Return;
 using testing::_;
+using testing::NiceMock;
+using testing::Return;
 
 namespace {
 
-class MockSearchIPCRouterDelegate : public SearchIPCRouter::Delegate {
- public:
-  virtual ~MockSearchIPCRouterDelegate() {}
-
-  MOCK_METHOD1(FocusOmnibox, void(bool focus));
-  MOCK_METHOD1(OnDeleteMostVisitedItem, void(const GURL& url));
-  MOCK_METHOD1(OnUndoMostVisitedDeletion, void(const GURL& url));
-  MOCK_METHOD0(OnUndoAllMostVisitedDeletions, void());
-  MOCK_METHOD2(OnAddCustomLink,
-               bool(const GURL& url, const std::string& title));
-  MOCK_METHOD3(OnUpdateCustomLink,
-               bool(const GURL& url,
-                    const GURL& new_url,
-                    const std::string& new_title));
-  MOCK_METHOD2(OnReorderCustomLink, bool(const GURL& url, int new_pos));
-  MOCK_METHOD1(OnDeleteCustomLink, bool(const GURL& url));
-  MOCK_METHOD0(OnUndoCustomLinkAction, void());
-  MOCK_METHOD0(OnResetCustomLinks, void());
-  MOCK_METHOD2(OnLogEvent, void(NTPLoggingEventType event,
-                                base::TimeDelta time));
-  MOCK_METHOD1(OnLogMostVisitedImpression,
-               void(const ntp_tiles::NTPTileImpression& impression));
-  MOCK_METHOD1(OnLogMostVisitedNavigation,
-               void(const ntp_tiles::NTPTileImpression& impression));
-  MOCK_METHOD1(PasteIntoOmnibox, void(const base::string16&));
-  MOCK_METHOD1(ChromeIdentityCheck, bool(const base::string16& identity));
-  MOCK_METHOD0(HistorySyncCheck, bool());
-  MOCK_METHOD1(OnSetCustomBackgroundURL, void(const GURL& url));
-  MOCK_METHOD4(OnSetCustomBackgroundURLWithAttributions,
-               void(const GURL& background_url,
-                    const std::string& attribution_line_1,
-                    const std::string& attribution_line_2,
-                    const GURL& action_url));
-  MOCK_METHOD0(OnSelectLocalBackgroundImage, void());
-};
-
 class MockEmbeddedSearchClientFactory
     : public SearchIPCRouter::EmbeddedSearchClientFactory {
  public:
@@ -104,7 +56,8 @@ class SearchTabHelperTest : public ChromeRenderViewHostTestHarness {
         std::make_unique<IdentityTestEnvironmentProfileAdaptor>(profile());
     SearchTabHelper::CreateForWebContents(web_contents());
     auto* search_tab = SearchTabHelper::FromWebContents(web_contents());
-    auto factory = std::make_unique<MockEmbeddedSearchClientFactory>();
+    auto factory =
+        std::make_unique<NiceMock<MockEmbeddedSearchClientFactory>>();
     ON_CALL(*factory, GetEmbeddedSearchClient())
         .WillByDefault(Return(&mock_embedded_search_client_));
     search_tab->ipc_router_for_testing()
@@ -144,16 +97,13 @@ class SearchTabHelperTest : public ChromeRenderViewHostTestHarness {
 
     syncer::ModelTypeSet result;
     if (sync_history) {
+      result.Put(syncer::TYPED_URLS);
       result.Put(syncer::HISTORY_DELETE_DIRECTIVES);
+      result.Put(syncer::SESSIONS);
     }
-    EXPECT_CALL(*sync_service, GetPreferredDataTypes())
-        .WillRepeatedly(Return(result));
-  }
-
-  MockSearchIPCRouterDelegate* mock_delegate() { return &delegate_; }
-
-  MockEmbeddedSearchClient* mock_embedded_search_client() {
-    return &mock_embedded_search_client_;
+    ON_CALL(*sync_service, IsFirstSetupComplete()).WillByDefault(Return(true));
+    ON_CALL(*sync_service, GetPreferredDataTypes())
+        .WillByDefault(Return(result));
   }
 
   identity::IdentityTestEnvironment* identity_test_env() {
@@ -162,8 +112,7 @@ class SearchTabHelperTest : public ChromeRenderViewHostTestHarness {
   }
 
  private:
-  MockSearchIPCRouterDelegate delegate_;
-  MockEmbeddedSearchClient mock_embedded_search_client_;
+  NiceMock<MockEmbeddedSearchClient> mock_embedded_search_client_;
   std::unique_ptr<IdentityTestEnvironmentProfileAdaptor>
       identity_test_env_adaptor_;
 };