Skip to content

Commit

Permalink
Don't instantiate ContentSuggestionsService on non-Android
Browse files Browse the repository at this point in the history
BUG=688366

Review-Url: https://codereview.chromium.org/2686053002
Cr-Commit-Position: refs/heads/master@{#449293}
  • Loading branch information
treib authored and Commit bot committed Feb 9, 2017
1 parent c53be51 commit c18c1ca
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 0 deletions.
22 changes: 22 additions & 0 deletions chrome/browser/ntp_snippets/content_suggestions_service_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,18 @@ using suggestions::ImageDecoderImpl;
using syncer::SyncService;
using translate::LanguageModel;

// For now, ContentSuggestionsService must only be instantiated on Android.
// See also crbug.com/688366.
#if defined(OS_ANDROID)
#define CONTENT_SUGGESTIONS_ENABLED 1
#else
#define CONTENT_SUGGESTIONS_ENABLED 0
#endif // OS_ANDROID

// The actual #if that does the work is below in BuildServiceInstanceFor. This
// one is just required to avoid "unused code" compiler errors.
#if CONTENT_SUGGESTIONS_ENABLED

namespace {

#if defined(OS_ANDROID)
Expand Down Expand Up @@ -124,6 +136,7 @@ void RegisterDownloadsProvider(OfflinePageModel* offline_page_model,
pref_service, base::MakeUnique<base::DefaultClock>());
service->RegisterProvider(std::move(provider));
}

#endif // OS_ANDROID

void RegisterBookmarkProvider(BookmarkModel* bookmark_model,
Expand All @@ -150,6 +163,7 @@ void RegisterPhysicalWebPageProvider(
service, physical_web_data_source, pref_service);
service->RegisterProvider(std::move(provider));
}

#endif // OS_ANDROID

void RegisterArticleProvider(SigninManagerBase* signin_manager,
Expand Down Expand Up @@ -213,6 +227,8 @@ void RegisterForeignSessionsProvider(SyncService* sync_service,

} // namespace

#endif // CONTENT_SUGGESTIONS_ENABLED

// static
ContentSuggestionsServiceFactory*
ContentSuggestionsServiceFactory::GetInstance() {
Expand Down Expand Up @@ -251,6 +267,8 @@ ContentSuggestionsServiceFactory::~ContentSuggestionsServiceFactory() = default;

KeyedService* ContentSuggestionsServiceFactory::BuildServiceInstanceFor(
content::BrowserContext* context) const {
#if CONTENT_SUGGESTIONS_ENABLED

using State = ContentSuggestionsService::State;
Profile* profile = Profile::FromBrowserContext(context);
DCHECK(!profile->IsOffTheRecord());
Expand Down Expand Up @@ -329,4 +347,8 @@ KeyedService* ContentSuggestionsServiceFactory::BuildServiceInstanceFor(
}

return service;

#else
return nullptr;
#endif // CONTENT_SUGGESTIONS_ENABLED
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright 2017 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 "chrome/browser/ntp_snippets/content_suggestions_service_factory.h"

#include "chrome/browser/ui/browser.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "testing/gtest/include/gtest/gtest.h"

using ContentSuggestionsServiceFactoryTest = InProcessBrowserTest;

IN_PROC_BROWSER_TEST_F(ContentSuggestionsServiceFactoryTest,
CreatesServiceOnlyOnAndroid) {
ntp_snippets::ContentSuggestionsService* service =
ContentSuggestionsServiceFactory::GetForProfile(browser()->profile());
#if defined(OS_ANDROID)
EXPECT_TRUE(service);
#else
EXPECT_FALSE(service);
#endif
}
1 change: 1 addition & 0 deletions chrome/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1654,6 +1654,7 @@ test("browser_tests") {
"../browser/net/sdch_browsertest.cc",
"../browser/net/spdyproxy/chrome_data_use_group_browsertest.cc",
"../browser/net/websocket_browsertest.cc",
"../browser/ntp_snippets/content_suggestions_service_factory_browsertest.cc",
"../browser/page_load_metrics/observers/https_engagement_metrics/https_engagement_page_load_metrics_observer_browsertest.cc",
"../browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.cc",
"../browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.h",
Expand Down

0 comments on commit c18c1ca

Please sign in to comment.