jkrcal@: are the suggestions updated when the device local changes?

On Android yes but only thanks to how Chrome on Android works. If you change system language and switch back to Chrome:
 - (Chrome performs a cold start)
 - Because preferred language is changed, Chrome clears cache which in turn clears cached suggestions
https://cs.chromium.org/chromium/src/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc?l=925
 - clearing cached suggestions in turn forces a refetch, with the new language settings.

What happens on iOS? Do we clear cache on iOS when language gets changed?

iOS also restarts all apps. I don't think there is a clear cache when the language gets changed. I will have to implements one.
Pushing it to M63 as it is minor and I won't have time.

ClearAllCachedSuggestions() does not seem to work. Even if I call it before showing the NTP, the first NTP has the previous suggestions, and it does not trigger a refresh.
Is there something I am missing?

ping jkrcal@.

I am sorry for the delay!

I do not understand why you get the old suggestions. Looking at https://cs.chromium.org/chromium/src/components/ntp_snippets/content_suggestions_service.cc?l=324, it does two things:
 - deletes the cache at the service (suggestions_by_category_)
 - notifies all observers that there are new suggestions for each category.

Q:
 - Do you listen to these notifications or you just call GetSuggestionsForCategory when the new NTP gets opened? 
 - Are you sure we do not trigger another fetch silently and somehow still ask for the previous UI language? 

When the NTP gets opened I call GetSuggestionsForCategory, for the available categories. But I also listen to notification of new suggestions.
By calling ClearAllCachedSuggestions I would expect GetSuggestionsForCategory to return an empty result, then get notified of the new suggestions. I don't understand how I can get the old suggestions after calling this.

I found the problem: the remote suggestions will do nothing if they are not initialized: https://cs.chromium.org/chromium/src/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc?sq=package:chromium&dr=CSs&l=599
I don't think this should be the expected behavior.

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

commit c34294c60676b8abc3dd2d7a384e30914d98bb76
Author: Gauthier Ambard <gambard@chromium.org>
Date: Wed Sep 27 09:56:00 2017

Do not add suggestions for initializing categories

If the category is not yet initialized, its suggestions should not be
shown.

Bug:  765164 
Change-Id: Iadc75ca2bc6b83856060fbe9323da2875bbe4e9f
Reviewed-on: https://chromium-review.googlesource.com/686418
Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504616}
[modify] https://crrev.com/c34294c60676b8abc3dd2d7a384e30914d98bb76/ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm

Cool! I am not sure about the state now. Is it fixed? Is there some fix needed in the component?

Sorry, the CL was unrelated to the initializing problem. The UI wasn't updating correctly if the component sends a notification for new suggestions while the status of the category is "initializing".
Either a fix is needed in the component or I can clear the suggestions later in the startup process (once remote suggestions is available). The first option seems better, WDYT?

Issue 775900 has been merged into this issue.

jkrcal@: I am able to clear the suggestions during the app init when the language is changed. The suggestions are removed but it looks like no new suggestions are fetched.
Is it expected?

No, this is not expected.

I am not able to reproduce this behavior on Android. Which is good but it also makes it harder to understand what's wrong. 

Unfortunately, from tomorrow on, I am OOO until Nov 06. How urgent is it?

(+vitaliii@ as a backup if this is really urgent)

This is not urgent and it can wait until your return :)
Thanks!

bumping this (still not urgent).

Indeed, I am back. I have a few high-prio tasks for this week which I look at first. I plan to fix this before M64 BP. Merging to M63 seems unrealistic at this stage. Does it SG?

My next step is to investigate more how it works on Android so that I can ask you more specific question about your implementation for iOS.

Sure, no problem. Thanks for looking into this!

Okay, it turns out this is broken on Android as well. 

I created a patch https://chromium-review.googlesource.com/c/chromium/src/+/793532. Could you please test it out on iOS that this fixes the bug?

LG. With your fix it is working. Thanks :)

Cool, thanks a lot for testing it!

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

commit 33ed751bf72f565332c437f991c07063716f5983
Author: Jan Krcal <jkrcal@chromium.org>
Date: Wed Nov 29 12:45:09 2017

[Remote suggestions] Fully reload suggestions after clearing cache

This CL fixes a bug that after changing UI language (which causes the
cache to get cleared), suggestions got not reloaded in the new lang.

The same functionality to reload suggestions worked well for internal
events of the RemoteSuggestionsProviderImpl such as signing in/out. The
problem was quite subtle:
 - the service used an API function that clears cache per category,
 - the provider itself used a "high-level" function that clears all
   suggestions at once and does all the extra logic to reload
   suggestions. It is hard to extract the extra logic into the
   low-level API function because this is shared with ClearHistory
   which should not reload suggestions.

Thus, this CL changes the service to call the high-level function,
instead. This keeps the code the easiest to read and argue about.

Concretely, this CL:
 - makes a trivial change in the ContentSuggestionProvider interface
  (and many its implementations), cache can be now cleared only on the
  per-provider granularity;
 - changes ContentSuggestionService to make use of this change;
 - changes the snippets-internals page (the only previous user of the
   finer per-category granularity) to clear all cache at once.

Bug:  765164 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ibe4ba60765d3c44fe917e1c6ff58e54cdd9a75ba
Reviewed-on: https://chromium-review.googlesource.com/793532
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520083}
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/chrome/browser/android/ntp/content_suggestions_notifier_service_unittest.cc
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/chrome/browser/ntp_snippets/download_suggestions_provider.cc
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/chrome/browser/ntp_snippets/download_suggestions_provider.h
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/chrome/browser/resources/snippets_internals.html
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/chrome/browser/resources/snippets_internals.js
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/chrome/browser/ui/webui/snippets_internals_message_handler.cc
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/components/ntp_snippets/bookmarks/bookmark_suggestions_provider.cc
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/components/ntp_snippets/bookmarks/bookmark_suggestions_provider.h
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/components/ntp_snippets/content_suggestions_provider.h
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/components/ntp_snippets/content_suggestions_service.cc
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/components/ntp_snippets/content_suggestions_service.h
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/components/ntp_snippets/mock_content_suggestions_provider.h
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/components/ntp_snippets/offline_pages/recent_tab_suggestions_provider.cc
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/components/ntp_snippets/offline_pages/recent_tab_suggestions_provider.h
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider.cc
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider.h
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/components/ntp_snippets/reading_list/reading_list_suggestions_provider.cc
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/components/ntp_snippets/reading_list/reading_list_suggestions_provider.h
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/components/ntp_snippets/remote/remote_suggestions_provider_impl.h
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/components/ntp_snippets/remote/remote_suggestions_scheduler_impl_unittest.cc
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/components/ntp_snippets/sessions/foreign_sessions_suggestions_provider.cc
[modify] https://crrev.com/33ed751bf72f565332c437f991c07063716f5983/components/ntp_snippets/sessions/foreign_sessions_suggestions_provider.h

Taking this back. I still need to land a CL (https://chromium-review.googlesource.com/c/chromium/src/+/737631) before it is fixed :)
Thanks for the fix!

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

commit 7063e0ac2b747f1be5c70d213e18bb2922323e58
Author: Gauthier Ambard <gambard@chromium.org>
Date: Wed Nov 29 14:56:06 2017

Clear suggestions when language changes

When the language of the device is changed, the suggestions should be
displayed in the new language.
This CL clears the cached suggestions when the language is changed.

Bug:  765164 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I12894c18531c8d1771d6c7e0d2b6c558ba9ce110
Reviewed-on: https://chromium-review.googlesource.com/737631
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520109}
[modify] https://crrev.com/7063e0ac2b747f1be5c70d213e18bb2922323e58/ios/chrome/app/main_controller.mm
[modify] https://crrev.com/7063e0ac2b747f1be5c70d213e18bb2922323e58/ios/chrome/browser/metrics/previous_session_info.h
[modify] https://crrev.com/7063e0ac2b747f1be5c70d213e18bb2922323e58/ios/chrome/browser/metrics/previous_session_info.mm
[modify] https://crrev.com/7063e0ac2b747f1be5c70d213e18bb2922323e58/ios/chrome/browser/metrics/previous_session_info_unittest.mm

Verified on chrome beta version 64.0.3282.75 on iPhone 8 plus with iOS 11.2.5 beta 3 and iPad Mini with iOS 11.1, following the steps mentioned in comment #0. Content suggestion links are translated as per device locale.  Looks good.