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

Issue 768410 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android , iOS
Pri: 3
Type: Task



Sign in to add a comment

[Content suggestions observer] Combine notifications onCategoryStatusChanged and onNewSuggestions

Project Member Reported by jkrcal@chromium.org, Sep 25 2017

Issue description

The ContentSuggestionsService::Observer interface currently has these two main functions for updates:
 - void OnNewSuggestions(Category category);
 - void OnCategoryStatusChanged(Category category,
                                CategoryStatus new_status);

Status and suggestions for a category can change at the same time and they can also change independently. Say, if a status change comes to the observer (e.g., UI), it is not clear whether newSuggestions will also arrive immediately afterwards or not. It is not even clear if the status change comes first or the newSuggestions notification comes first.

This adds unnecessary complexity and brittleness to the implementation of the observers.

We should fix it.

Maybe by adding a third function 
 - void onCategoryStatusAndSuggestionsChanged(Category category, CategoryStatus new_status)?

Nicolas, Gauthier, do you have a better proposal?
 

Comment 1 by fi...@chromium.org, Sep 25 2017

Labels: zine-triaged
I think having only two is fine. Because you can always have the case where they are called independently at the same time (i.e. status changes and suggestions change independently but still between two drawing of the UI). So the UI would need to be able to cope with it in any case.
Components: -UI>Browser>NewTabPage UI>Browser>ContentSuggestions
Labels: -Type-Feature Type-Task

Comment 4 by jkrcal@chromium.org, Nov 28 2017

Status: WontFix (was: Assigned)
I suggest to drop this plan, due to jardin.

Feel free to reopen if you think this is still important to change now.

Sign in to add a comment