New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 633139 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Email to this user bounced
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android , iOS
Pri: 1
Type: Feature

Blocked on:
issue 632690

Blocking:
issue 637250



Sign in to add a comment

ContentSuggestionsServiceFactory and Providers refactoring

Project Member Reported by pke@google.com, Aug 1 2016

Issue description

Providers should not be KeyedServices anymore, instead be owned by the ContentSuggestionsService and be created by the service's factory (both Android and iOS).
 
Project Member

Comment 1 by sheriffbot@chromium.org, Aug 1 2016

Labels: Hotlist-Google

Comment 2 by fi...@chromium.org, Aug 1 2016

Labels: zine-client-sections-v1
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 3 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5728f0895e82f74b307d39db57099a5197ebfeb0

commit 5728f0895e82f74b307d39db57099a5197ebfeb0
Author: pke <pke@google.com>
Date: Wed Aug 03 17:27:35 2016

Combine all suggestions factories into ContentSuggestionsServiceFactory

Remove the NTPSnippetsServiceObserver and its uses.
Change ContentSuggestionsProviders to be owned by the
ContentSuggestionsService instead of having their own lifecycle as a
KeyedService. Change RegisterProvider() and Shutdown() behavior for
the service and the providers.
Remove NTPSnippetsServiceFactory and its uses in the SnippetsBridge.
Remove OfflinePageSuggestionsProviderFactory.
Expose the NTPSnippetsService to the snippets-internals through the
ContentSuggestionsService.
Adjust unit tests.

BUG= 633139 

Review-Url: https://codereview.chromium.org/2205233002
Cr-Commit-Position: refs/heads/master@{#409556}

[modify] https://crrev.com/5728f0895e82f74b307d39db57099a5197ebfeb0/chrome/browser/android/ntp/ntp_snippets_bridge.cc
[delete] https://crrev.com/ca02e0e5c33ca26f201fd3deaa738ee91a3351a7/chrome/browser/android/ntp/offline_page_suggestions_provider_factory.cc
[delete] https://crrev.com/ca02e0e5c33ca26f201fd3deaa738ee91a3351a7/chrome/browser/android/ntp/offline_page_suggestions_provider_factory.h
[modify] https://crrev.com/5728f0895e82f74b307d39db57099a5197ebfeb0/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc
[delete] https://crrev.com/ca02e0e5c33ca26f201fd3deaa738ee91a3351a7/chrome/browser/ntp_snippets/ntp_snippets_service_factory.cc
[delete] https://crrev.com/ca02e0e5c33ca26f201fd3deaa738ee91a3351a7/chrome/browser/ntp_snippets/ntp_snippets_service_factory.h
[modify] https://crrev.com/5728f0895e82f74b307d39db57099a5197ebfeb0/chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc
[modify] https://crrev.com/5728f0895e82f74b307d39db57099a5197ebfeb0/chrome/browser/profiles/profile_manager.cc
[modify] https://crrev.com/5728f0895e82f74b307d39db57099a5197ebfeb0/chrome/browser/ui/webui/snippets_internals_message_handler.cc
[modify] https://crrev.com/5728f0895e82f74b307d39db57099a5197ebfeb0/chrome/browser/ui/webui/snippets_internals_message_handler.h
[modify] https://crrev.com/5728f0895e82f74b307d39db57099a5197ebfeb0/chrome/chrome_browser.gypi
[modify] https://crrev.com/5728f0895e82f74b307d39db57099a5197ebfeb0/components/ntp_snippets/content_suggestions_provider.cc
[modify] https://crrev.com/5728f0895e82f74b307d39db57099a5197ebfeb0/components/ntp_snippets/content_suggestions_provider.h
[modify] https://crrev.com/5728f0895e82f74b307d39db57099a5197ebfeb0/components/ntp_snippets/content_suggestions_service.cc
[modify] https://crrev.com/5728f0895e82f74b307d39db57099a5197ebfeb0/components/ntp_snippets/content_suggestions_service.h
[modify] https://crrev.com/5728f0895e82f74b307d39db57099a5197ebfeb0/components/ntp_snippets/content_suggestions_service_unittest.cc
[modify] https://crrev.com/5728f0895e82f74b307d39db57099a5197ebfeb0/components/ntp_snippets/ntp_snippets_service.cc
[modify] https://crrev.com/5728f0895e82f74b307d39db57099a5197ebfeb0/components/ntp_snippets/ntp_snippets_service.h
[modify] https://crrev.com/5728f0895e82f74b307d39db57099a5197ebfeb0/components/ntp_snippets/ntp_snippets_service_unittest.cc
[modify] https://crrev.com/5728f0895e82f74b307d39db57099a5197ebfeb0/components/ntp_snippets/offline_pages/offline_page_suggestions_provider.cc
[modify] https://crrev.com/5728f0895e82f74b307d39db57099a5197ebfeb0/components/ntp_snippets/offline_pages/offline_page_suggestions_provider.h

Comment 4 by pke@google.com, Aug 5 2016

Status: Started (was: Assigned)

Comment 5 by pke@google.com, Aug 8 2016

Labels: zine-16-08-08

Comment 6 by pke@google.com, Aug 12 2016

Blocking: 637250

Comment 7 by pke@google.com, Aug 12 2016

Status: Fixed (was: Started)
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 25 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c9f49e317107c0a827f559f4a2da1050dd207358

commit c9f49e317107c0a827f559f4a2da1050dd207358
Author: pke <pke@google.com>
Date: Thu Aug 25 13:21:37 2016

Register all providers in IOSChromeContentSuggestionsServiceFactory

Create the articles provider (NTPSnippetsService) and the
BookmarkSuggestionsProvider in the
IOSChromeContentSuggestionsServiceFactory and register them with
the ContentSuggestionsService.

This makes the IOSChromeContentSuggestionsServiceFactory equivalent
to the ContentSuggestionsServiceFactory again (apart from the
OfflinePageSuggestionsProvider, which is Android-only).

BUG= 633139 

Review-Url: https://codereview.chromium.org/2232473002
Cr-Commit-Position: refs/heads/master@{#414415}

[modify] https://crrev.com/c9f49e317107c0a827f559f4a2da1050dd207358/ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 25 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/496451c110e32151e4ce05dc4e2e0ca83c69a3df

commit 496451c110e32151e4ce05dc4e2e0ca83c69a3df
Author: vitaliii <vitaliii@chromium.org>
Date: Thu Aug 25 18:22:38 2016

ContentSuggestionsService providers registration refactoring.

BUG= 633139 

Review-Url: https://codereview.chromium.org/2245633002
Cr-Commit-Position: refs/heads/master@{#414481}

[modify] https://crrev.com/496451c110e32151e4ce05dc4e2e0ca83c69a3df/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc

Sign in to add a comment