Avoid refreshing the whole NTP for every change to the sections |
|
Issue descriptionWe currently refresh the whole NTP in a few situations that do not require it: - The status of a section changed - New suggestions have been added - A suggestion is invalidated - A section is dismissed (usages of NTPAdapter#updateGroups()) Changes are restricted to a section, so we should refresh only that section, and maybe the bottom spacer. The sections should be able to report to the adapter about changes that happened with them and provide a range of positions to update. This should also fix the problem with NTPAdapter#getItems() that we need to call quite often and involves recreating item lists every time.
,
Sep 29 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/509bf6e55d629763e4ba67a123c575de1d60c686 commit 509bf6e55d629763e4ba67a123c575de1d60c686 Author: dgn <dgn@chromium.org> Date: Thu Sep 29 12:47:33 2016 [NTP Client] Stop refreshing the whole NTP for every adapter change Replace NTPAdapter#updateGroups() with more granular notifications. Note: We are using SuggestionSections#getItems() quite often, and that creates new lists every time, so it could still be improved. Preview: https://goo.gl/photos/UHWg4pfbkWWPHdGi8 BUG= 647304 ,643106,647671 Review-Url: https://codereview.chromium.org/2360813004 Cr-Commit-Position: refs/heads/master@{#421806} [modify] https://crrev.com/509bf6e55d629763e4ba67a123c575de1d60c686/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java [modify] https://crrev.com/509bf6e55d629763e4ba67a123c575de1d60c686/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ItemGroup.java [modify] https://crrev.com/509bf6e55d629763e4ba67a123c575de1d60c686/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java [modify] https://crrev.com/509bf6e55d629763e4ba67a123c575de1d60c686/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java [modify] https://crrev.com/509bf6e55d629763e4ba67a123c575de1d60c686/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/FakeSuggestionsSource.java [modify] https://crrev.com/509bf6e55d629763e4ba67a123c575de1d60c686/chrome/android/java_sources.gni [modify] https://crrev.com/509bf6e55d629763e4ba67a123c575de1d60c686/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/snippets/ArticleSnippetsTest.java [add] https://crrev.com/509bf6e55d629763e4ba67a123c575de1d60c686/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/ContentSuggestionsTestUtils.java [modify] https://crrev.com/509bf6e55d629763e4ba67a123c575de1d60c686/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java [add] https://crrev.com/509bf6e55d629763e4ba67a123c575de1d60c686/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java
,
Oct 7 2016
Is there anything left to do?
,
Oct 7 2016
Yes, there's still a call to notifyDataSetChanged() in NewTabPageAdapter#resetSections().
,
Oct 7 2016
I think that one can stay, it's used when the adapter is initialised and has no visible effect anyway. I left it open for the data structure changes... We still do repeated calls to getItems() in regular Adapter operations. They make us recreate many lists to gather the items and then throw them away. bauerb@ had a WIP CL to avoid that. Not sure if he wants that work to go under this CL. If not we can close it.
,
Oct 7 2016
When refreshing from the all dismissed state, we'll run this code also.
,
Oct 7 2016
...which should also be fine, because we wouldn't have many elements in that case either?
,
Oct 7 2016
Just the above the fold item and the bottom spacer. It would still be nicer to have animations rather than a flash there, when the new sections are inserted.
,
Nov 21 2016
|
|
►
Sign in to add a comment |
|
Comment 1 by fi...@chromium.org
, Sep 19 2016