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

Issue 616090 link

Starred by 4 users

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocked on:
issue 610692
issue 647304



Sign in to add a comment

NTP Clank Refactor / Multi-Section UI

Project Member Reported by mcwilliams@chromium.org, May 31 2016

Issue description

Refactor the NTP Clank so java/xml

Rewrite/refactor the New Tab Page (NTP) java/xml code which covers the NTP Chrome on Android app. This is currently being used on both mobile and tablet. Make the NTP scalable to include alternative information sources for the user to interact with.

Design doc being created now and will be linked.
 
Blockedon: 610692
Labels: zine-mr-iter-17
For the purposes of this bug: is it fair to say that one of the main goals is going to be to support multiple Zine sections with separate lists of snippets?
Status: Assigned (was: Untriaged)
That's correct.

(Changing to Assigned because there's an owner.)
Labels: zine-mr-iter-19
Labels: zine-mr-iter-20
Labels: zine-mr-iter-21

Comment 8 by fi...@chromium.org, Jul 1 2016

Labels: zine-16-06-27
Cc: mvanouwe...@chromium.org
Labels: -Pri-3 Pri-2
Bumping priority as the multi section feature is coming up. Here's the design document (currently in draft status): https://goo.gl/xYFiq2
Labels: zine-16-07-18
Cc: mcwilliams@chromium.org
Owner: mvanouwe...@chromium.org
Project Member

Comment 12 by bugdroid1@chromium.org, Jul 19 2016

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

commit 96d9e341ccfaff04bd3f0d6163c47ee17e567485
Author: mvanouwerkerk <mvanouwerkerk@chromium.org>
Date: Tue Jul 19 09:58:23 2016

Break smaller methods out of NewTabPage#initialize which was ~150 lines.

BUG=616090

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

[modify] https://crrev.com/96d9e341ccfaff04bd3f0d6163c47ee17e567485/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java

Labels: zine-16-07-25
Status: Started (was: Assigned)
Project Member

Comment 19 by bugdroid1@chromium.org, Jul 25 2016

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

commit 41bf504929935a37fb782decd602d565c9409923
Author: mvanouwerkerk <mvanouwerkerk@chromium.org>
Date: Mon Jul 25 17:11:17 2016

Make NewTabPageAdapter the only source of truth for adapter item positions.

Also while I'm here:
* Eliminate some hard coded position calculations
* Contain the remaining position calculations in helper functions
* Rename some members for consistency and clarity
* Delete unused NewTabPageRecyclerView#getLinearLayoutManager

BUG=616090

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

[modify] https://crrev.com/41bf504929935a37fb782decd602d565c9409923/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
[modify] https://crrev.com/41bf504929935a37fb782decd602d565c9409923/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java

Comment 20 by pke@google.com, Jul 26 2016

Summary: NTP Clank Refactor / Multi-Section UI (was: NTP Clank Refactor)
Project Member

Comment 21 by bugdroid1@chromium.org, Jul 27 2016

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

commit cc3ed5d9ca70635cf707d94cbdac78f9902b96ef
Author: pke <pke@google.com>
Date: Wed Jul 27 08:47:08 2016

Show content suggestions from all categories on the NTP

Currently, the NTP UI only supports a single section of content
suggestions. Therefore, the available content suggestions from all
categories are displayed in a single category to allow
experimenting with new ContentSuggestionProviders until the UI supports separate sections.

BUG= 619560 ,616090

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

[modify] https://crrev.com/cc3ed5d9ca70635cf707d94cbdac78f9902b96ef/chrome/browser/android/ntp/ntp_snippets_bridge.cc
[modify] https://crrev.com/cc3ed5d9ca70635cf707d94cbdac78f9902b96ef/components/ntp_snippets/offline_pages/offline_page_suggestions_provider.cc

Labels: zine-triaged zine-16-08-01
Owner: bauerb@chromium.org
Labels: -Pri-2 M-54 Pri-1
Adjusting labels. This is obviously one of the blockers for our M54 rollout. :)
Project Member

Comment 24 by bugdroid1@chromium.org, Aug 5 2016

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

commit 57a839dc61aece5b2138586dd0ffaa500b7ccc7f
Author: bauerb <bauerb@chromium.org>
Date: Fri Aug 05 16:30:11 2016

Zine: support multiple sections in the ui

* The adapter now manages groups of items.
* A SuggestionsSection is a group that holds suggestions, a header, and a status card.
* Add @SuggestionsCategory and @SuggestionsCategoryStatus annotations.
* SnippetsObserver now specifies what category it is talking about.

Based on http://codereview.chromium.org/2194433002#ps20001 by mvanouwerkerk@chromium.org.

BUG=616090

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

[modify] https://crrev.com/57a839dc61aece5b2138586dd0ffaa500b7ccc7f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/AboveTheFoldListItem.java
[add] https://crrev.com/57a839dc61aece5b2138586dd0ffaa500b7ccc7f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ItemGroup.java
[modify] https://crrev.com/57a839dc61aece5b2138586dd0ffaa500b7ccc7f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
[modify] https://crrev.com/57a839dc61aece5b2138586dd0ffaa500b7ccc7f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java
[add] https://crrev.com/57a839dc61aece5b2138586dd0ffaa500b7ccc7f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SingleItemGroup.java
[modify] https://crrev.com/57a839dc61aece5b2138586dd0ffaa500b7ccc7f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SpacingListItem.java
[modify] https://crrev.com/57a839dc61aece5b2138586dd0ffaa500b7ccc7f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusListItem.java
[add] https://crrev.com/57a839dc61aece5b2138586dd0ffaa500b7ccc7f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java
[modify] https://crrev.com/57a839dc61aece5b2138586dd0ffaa500b7ccc7f/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleListItem.java
[modify] https://crrev.com/57a839dc61aece5b2138586dd0ffaa500b7ccc7f/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java
[modify] https://crrev.com/57a839dc61aece5b2138586dd0ffaa500b7ccc7f/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsSource.java
[modify] https://crrev.com/57a839dc61aece5b2138586dd0ffaa500b7ccc7f/chrome/android/java_sources.gni
[modify] https://crrev.com/57a839dc61aece5b2138586dd0ffaa500b7ccc7f/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java
[modify] https://crrev.com/57a839dc61aece5b2138586dd0ffaa500b7ccc7f/chrome/browser/android/ntp/ntp_snippets_bridge.cc
[modify] https://crrev.com/57a839dc61aece5b2138586dd0ffaa500b7ccc7f/chrome/browser/android/ntp/ntp_snippets_bridge.h
[modify] https://crrev.com/57a839dc61aece5b2138586dd0ffaa500b7ccc7f/components/ntp_snippets/BUILD.gn
[modify] https://crrev.com/57a839dc61aece5b2138586dd0ffaa500b7ccc7f/components/ntp_snippets/category.h

Labels: zine-16-08-08
Labels: zine-client-sections-v1
Hi, is this bug still tracking something that is missing? The bug it is blocked on is also not clear w.r.t. what is missing. Shall we mark this fixed?
Blockedon: -610692
Blockedon: 610692
Cc: maybelle@chromium.org
 Issue 610692  has been merged into this issue.
The refactoring isn't fully done yet. I'd like to keep this open just for our internal tracking purposes (it's not a blocking issue).
Labels: zine-16-08-15
Project Member

Comment 32 by bugdroid1@chromium.org, Aug 15 2016

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

commit 3323db9ee62d0bc21278d904823413a7f2cff3a6
Author: bauerb <bauerb@chromium.org>
Date: Mon Aug 15 23:14:35 2016

Remove "list item" from Android NTP class names.

This is in preparation for more refactorings that will get rid of having an explicit list of NTP items.

BUG=616090

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

[modify] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
[rename] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/AboveTheFoldItem.java
[rename] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ActionItem.java
[modify] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/CardViewHolder.java
[modify] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ItemGroup.java
[modify] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
[rename] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageItem.java
[modify] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java
[modify] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageViewHolder.java
[rename] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ProgressItem.java
[modify] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ProgressViewHolder.java
[modify] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SingleItemGroup.java
[rename] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SpacingItem.java
[rename] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusItem.java
[modify] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java
[add] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SectionHeader.java
[rename] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SectionHeaderViewHolder.java
[rename] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java
[modify] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java
[delete] https://crrev.com/3cf02f522b6428e4bd30ac55da07657a3e55072c/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetHeaderListItem.java
[modify] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java
[modify] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SuggestionsSource.java
[modify] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/java_sources.gni
[modify] https://crrev.com/3323db9ee62d0bc21278d904823413a7f2cff3a6/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java

Labels: -Pri-1 -M-54 M-55 Pri-3
Moving to M55 at lower priority because this seems to be ongoing refactorings. Feel free to adjust if this is non-sense.
Project Member

Comment 34 by bugdroid1@chromium.org, Sep 8 2016

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

commit 42d87c5d7b85ba9d6df6bcaa8b1f7830e5e8430f
Author: bauerb <bauerb@chromium.org>
Date: Thu Sep 08 14:43:31 2016

Move onBindViewHolder from NewTabPageViewHolder to NewTabPageItem.

This moves NewTabPageItem further away from being a generic base class for holding the data shown in the RecyclerView, and will allow pushing the responsibility to update the view into the merged item/group interface.

BUG=616090

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

[modify] https://crrev.com/42d87c5d7b85ba9d6df6bcaa8b1f7830e5e8430f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/AboveTheFoldItem.java
[modify] https://crrev.com/42d87c5d7b85ba9d6df6bcaa8b1f7830e5e8430f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ActionItem.java
[modify] https://crrev.com/42d87c5d7b85ba9d6df6bcaa8b1f7830e5e8430f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/CardViewHolder.java
[modify] https://crrev.com/42d87c5d7b85ba9d6df6bcaa8b1f7830e5e8430f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/Footer.java
[modify] https://crrev.com/42d87c5d7b85ba9d6df6bcaa8b1f7830e5e8430f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
[modify] https://crrev.com/42d87c5d7b85ba9d6df6bcaa8b1f7830e5e8430f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageItem.java
[modify] https://crrev.com/42d87c5d7b85ba9d6df6bcaa8b1f7830e5e8430f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageViewHolder.java
[modify] https://crrev.com/42d87c5d7b85ba9d6df6bcaa8b1f7830e5e8430f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ProgressItem.java
[modify] https://crrev.com/42d87c5d7b85ba9d6df6bcaa8b1f7830e5e8430f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ProgressViewHolder.java
[modify] https://crrev.com/42d87c5d7b85ba9d6df6bcaa8b1f7830e5e8430f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SpacingItem.java
[modify] https://crrev.com/42d87c5d7b85ba9d6df6bcaa8b1f7830e5e8430f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusItem.java
[modify] https://crrev.com/42d87c5d7b85ba9d6df6bcaa8b1f7830e5e8430f/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SectionHeader.java
[modify] https://crrev.com/42d87c5d7b85ba9d6df6bcaa8b1f7830e5e8430f/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SectionHeaderViewHolder.java
[modify] https://crrev.com/42d87c5d7b85ba9d6df6bcaa8b1f7830e5e8430f/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java
[modify] https://crrev.com/42d87c5d7b85ba9d6df6bcaa8b1f7830e5e8430f/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java

Labels: zine-16-09-12
Hi Bernhard, shall we mark this as fixed in M55, or move to M56?
Labels: -M-55 M-56
Punt to M56.

We could also remove the milestone completely, as this is a completely user-invisible refactoring, so it doesn't really matter in which release the final state will show up.
(Or alternatively, split the pure multi-section UI from the refactoring, and mark the former as fixed in M54).
Project Member

Comment 40 by bugdroid1@chromium.org, Oct 7 2016

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

commit 1660bf28e21367fddd506158781f0ab190dc292f
Author: bauerb <bauerb@chromium.org>
Date: Fri Oct 07 20:40:11 2016

NTP cards: Restructure change notifications.

This moves the logic for notifying about SuggestionSection updates into
SuggestionSection, so that the ItemGroup.Observer is agnostic of sections.

Also replaces the NewTabPageAdapter spy() in the NewTabPageAdapterTest with a
mocked AdapterDataObserver.

BUG=616090

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

[modify] https://crrev.com/1660bf28e21367fddd506158781f0ab190dc292f/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
[modify] https://crrev.com/1660bf28e21367fddd506158781f0ab190dc292f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ItemGroup.java
[modify] https://crrev.com/1660bf28e21367fddd506158781f0ab190dc292f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
[modify] https://crrev.com/1660bf28e21367fddd506158781f0ab190dc292f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SigninPromoItem.java
[modify] https://crrev.com/1660bf28e21367fddd506158781f0ab190dc292f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java
[modify] https://crrev.com/1660bf28e21367fddd506158781f0ab190dc292f/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/snippets/ArticleSnippetsTest.java
[modify] https://crrev.com/1660bf28e21367fddd506158781f0ab190dc292f/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java
[modify] https://crrev.com/1660bf28e21367fddd506158781f0ab190dc292f/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java

Project Member

Comment 41 by bugdroid1@chromium.org, Oct 7 2016

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

commit 1660bf28e21367fddd506158781f0ab190dc292f
Author: bauerb <bauerb@chromium.org>
Date: Fri Oct 07 20:40:11 2016

NTP cards: Restructure change notifications.

This moves the logic for notifying about SuggestionSection updates into
SuggestionSection, so that the ItemGroup.Observer is agnostic of sections.

Also replaces the NewTabPageAdapter spy() in the NewTabPageAdapterTest with a
mocked AdapterDataObserver.

BUG=616090

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

[modify] https://crrev.com/1660bf28e21367fddd506158781f0ab190dc292f/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
[modify] https://crrev.com/1660bf28e21367fddd506158781f0ab190dc292f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ItemGroup.java
[modify] https://crrev.com/1660bf28e21367fddd506158781f0ab190dc292f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
[modify] https://crrev.com/1660bf28e21367fddd506158781f0ab190dc292f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SigninPromoItem.java
[modify] https://crrev.com/1660bf28e21367fddd506158781f0ab190dc292f/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java
[modify] https://crrev.com/1660bf28e21367fddd506158781f0ab190dc292f/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/snippets/ArticleSnippetsTest.java
[modify] https://crrev.com/1660bf28e21367fddd506158781f0ab190dc292f/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java
[modify] https://crrev.com/1660bf28e21367fddd506158781f0ab190dc292f/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java

Labels: zine-16-10-10
Project Member

Comment 43 by bugdroid1@chromium.org, Oct 12 2016

Project Member

Comment 44 by bugdroid1@chromium.org, Oct 17 2016

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

commit 728cf24b0a3552c2b60ed39f82760ff6d18cc21a
Author: bauerb <bauerb@chromium.org>
Date: Mon Oct 17 15:48:14 2016

Unify NewTabPageItem and ItemGroup into a single tree-structured interface.

BUG=616090

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

[modify] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/AboveTheFoldItem.java
[modify] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ActionItem.java
[modify] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/AllDismissedItem.java
[modify] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/CardViewHolder.java
[add] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ChildNode.java
[modify] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/Footer.java
[add] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/InnerNode.java
[delete] https://crrev.com/a9ffa9ae8f544baf487c2204139fe716d40d021a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ItemGroup.java
[add] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ItemViewType.java
[add] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/Leaf.java
[modify] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
[delete] https://crrev.com/a9ffa9ae8f544baf487c2204139fe716d40d021a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageItem.java
[modify] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java
[add] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NodeParent.java
[modify] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ProgressItem.java
[modify] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ProgressViewHolder.java
[modify] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SignInPromo.java
[delete] https://crrev.com/a9ffa9ae8f544baf487c2204139fe716d40d021a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SingleItemGroup.java
[modify] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SpacingItem.java
[modify] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusItem.java
[modify] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java
[add] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/TreeNode.java
[modify] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SectionHeader.java
[modify] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java
[modify] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/java_sources.gni
[modify] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/ContentSuggestionsTestUtils.java
[add] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/InnerNodeTest.java
[modify] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java
[modify] https://crrev.com/728cf24b0a3552c2b60ed39f82760ff6d18cc21a/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java

Project Member

Comment 47 by bugdroid1@chromium.org, Oct 24 2016

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

commit 311b9cac063765ab69fb69f76c84bdeba8f722e3
Author: bauerb <bauerb@chromium.org>
Date: Mon Oct 24 13:46:46 2016

[Android NTP] Move suggestions in SuggestionSection into a separate TreeNode.

This makes SnippetArticle a pure model class that doesn't know about the adapter.

Goals of the overall refactoring:
1) Decoupling the various pieces of logic, by moving them from the adapter into
the tree structure. For example, dismissal is something that could be handled by
a TreeNode instead of doing the big switch statement we have right now in the
Adapter.
2) Stabilizing the structure of the tree. Right now we tend to throw away
everything and rebuild, which means we have to manually figure out what changes
exactly to notify about (cf. removeSuggestion). If we keep the tree structure
stable and just adjust the number of children below each node as necessary (like
SigninPromo already does), we get correct notifications for free, and it will
allow for some optimizations as well (we could cache the number of children for
each node and update it incrementally when a subtree changes. Right now we'd
have to contort ourselves a bit to make that work with things like
resetChildren()).

BUG=616090

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

[modify] https://crrev.com/311b9cac063765ab69fb69f76c84bdeba8f722e3/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java
[modify] https://crrev.com/311b9cac063765ab69fb69f76c84bdeba8f722e3/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java

Labels: -M-56
Removing milestone, as this is not a user-visible change.
Project Member

Comment 49 by bugdroid1@chromium.org, Nov 9 2016

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

commit 4e2a6810fd84198ce9d8062d69393076b48a91e9
Author: dgn <dgn@chromium.org>
Date: Wed Nov 09 14:50:23 2016

[NTP Client] Refactor SuggestionsSection change detection

The change detection and notification used to rely on keeping track
of children sizes, manually notifying on certain changes. This CL
finishes making its children OptionalLeaves, and makes
SuggestionList the only one capable of modifying the list of
suggestions. This allows letting these children notify of changes
themselves.

BUG=616090

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

[modify] https://crrev.com/4e2a6810fd84198ce9d8062d69393076b48a91e9/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ActionItem.java
[modify] https://crrev.com/4e2a6810fd84198ce9d8062d69393076b48a91e9/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
[modify] https://crrev.com/4e2a6810fd84198ce9d8062d69393076b48a91e9/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/OptionalLeaf.java
[modify] https://crrev.com/4e2a6810fd84198ce9d8062d69393076b48a91e9/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ProgressItem.java
[modify] https://crrev.com/4e2a6810fd84198ce9d8062d69393076b48a91e9/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SignInPromo.java
[modify] https://crrev.com/4e2a6810fd84198ce9d8062d69393076b48a91e9/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusItem.java
[modify] https://crrev.com/4e2a6810fd84198ce9d8062d69393076b48a91e9/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java
[modify] https://crrev.com/4e2a6810fd84198ce9d8062d69393076b48a91e9/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/ContentSuggestionsTestUtils.java
[modify] https://crrev.com/4e2a6810fd84198ce9d8062d69393076b48a91e9/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java
[modify] https://crrev.com/4e2a6810fd84198ce9d8062d69393076b48a91e9/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java

Comment 50 by dgn@chromium.org, Nov 21 2016

Blockedon: 647304
Project Member

Comment 51 by bugdroid1@chromium.org, Nov 21 2016

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

commit 6cf39f0878de2cc72b7631aa9d5a58f2310495a4
Author: bauerb <bauerb@chromium.org>
Date: Mon Nov 21 11:47:27 2016

Move fetching new suggestions for all categories to the SuggestionsSource.

This removes dependencies on NewTabPageAdapter in favor of SuggestionsSource,
which can be more easily mocked in tests.

BUG=616090

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

[modify] https://crrev.com/6cf39f0878de2cc72b7631aa9d5a58f2310495a4/chrome/android/java/src/org/chromium/chrome/browser/ChromeBackgroundService.java
[modify] https://crrev.com/6cf39f0878de2cc72b7631aa9d5a58f2310495a4/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ActionItem.java
[modify] https://crrev.com/6cf39f0878de2cc72b7631aa9d5a58f2310495a4/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/AllDismissedItem.java
[modify] https://crrev.com/6cf39f0878de2cc72b7631aa9d5a58f2310495a4/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
[modify] https://crrev.com/6cf39f0878de2cc72b7631aa9d5a58f2310495a4/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/FakeSuggestionsSource.java
[modify] https://crrev.com/6cf39f0878de2cc72b7631aa9d5a58f2310495a4/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java
[modify] https://crrev.com/6cf39f0878de2cc72b7631aa9d5a58f2310495a4/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SuggestionsSource.java
[modify] https://crrev.com/6cf39f0878de2cc72b7631aa9d5a58f2310495a4/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java

Project Member

Comment 53 by bugdroid1@chromium.org, Nov 25 2016

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

commit c8ecd6531500968e2babc28de6604e1887060358
Author: bauerb <bauerb@chromium.org>
Date: Fri Nov 25 13:26:56 2016

Remove warnings in org.chromium.chrome.browser.ntp.* code and JUnit tests.

BUG=616090

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

[modify] https://crrev.com/c8ecd6531500968e2babc28de6604e1887060358/chrome/android/java/src/org/chromium/chrome/browser/ntp/DisplayStyleObserver.java
[modify] https://crrev.com/c8ecd6531500968e2babc28de6604e1887060358/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
[modify] https://crrev.com/c8ecd6531500968e2babc28de6604e1887060358/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java
[modify] https://crrev.com/c8ecd6531500968e2babc28de6604e1887060358/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java
[modify] https://crrev.com/c8ecd6531500968e2babc28de6604e1887060358/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/ContentSuggestionsTestUtils.java
[modify] https://crrev.com/c8ecd6531500968e2babc28de6604e1887060358/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java
[modify] https://crrev.com/c8ecd6531500968e2babc28de6604e1887060358/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java

Project Member

Comment 54 by bugdroid1@chromium.org, Dec 5 2016

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

commit 6c65a58be01cfe2b054752b29f4b393887441e6d
Author: bauerb <bauerb@chromium.org>
Date: Mon Dec 05 17:06:33 2016

[Android NTP] Reduce API surface of NewTabPageAdapter.

Remove:
* getSignInPromoPosition()
* getSuggestionPosition()

Change to default visibility:
* getBottomSpacerPosition()
* getLastContentItemPosition()

Change to private:
* hasAllBeenDismissed()
* getSuggestionsSection()
* getChildPositionOffset()

Two methods are added for tests:
* getSectionForTesting()
* getRootForTesting()

BUG=616090

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

[modify] https://crrev.com/6c65a58be01cfe2b054752b29f4b393887441e6d/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
[modify] https://crrev.com/6c65a58be01cfe2b054752b29f4b393887441e6d/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java
[modify] https://crrev.com/6c65a58be01cfe2b054752b29f4b393887441e6d/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java
[modify] https://crrev.com/6c65a58be01cfe2b054752b29f4b393887441e6d/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java
[modify] https://crrev.com/6c65a58be01cfe2b054752b29f4b393887441e6d/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerViewTest.java
[modify] https://crrev.com/6c65a58be01cfe2b054752b29f4b393887441e6d/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java

Project Member

Comment 55 by bugdroid1@chromium.org, Dec 13 2016

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

commit 67606628f1bf101b485ed5fd1ef00fd452e306f7
Author: bauerb <bauerb@chromium.org>
Date: Tue Dec 13 12:05:34 2016

[Android NTP] Move suggestion sections into a separate node.

Also, define how initialization of nodes works, by adding an init()
method that is called after creating a node.

Initialization now happens after the full tree structure has been
created, and recursively processes all nodes in the tree. Subtrees that
are added later use the same pattern; InnerNode now has helper methods
for this.

BUG=616090

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

[modify] https://crrev.com/67606628f1bf101b485ed5fd1ef00fd452e306f7/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/AllDismissedItem.java
[modify] https://crrev.com/67606628f1bf101b485ed5fd1ef00fd452e306f7/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ChildNode.java
[modify] https://crrev.com/67606628f1bf101b485ed5fd1ef00fd452e306f7/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/InnerNode.java
[modify] https://crrev.com/67606628f1bf101b485ed5fd1ef00fd452e306f7/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/Leaf.java
[modify] https://crrev.com/67606628f1bf101b485ed5fd1ef00fd452e306f7/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
[add] https://crrev.com/67606628f1bf101b485ed5fd1ef00fd452e306f7/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SectionList.java
[modify] https://crrev.com/67606628f1bf101b485ed5fd1ef00fd452e306f7/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SignInPromo.java
[modify] https://crrev.com/67606628f1bf101b485ed5fd1ef00fd452e306f7/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java
[modify] https://crrev.com/67606628f1bf101b485ed5fd1ef00fd452e306f7/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/TreeNode.java
[modify] https://crrev.com/67606628f1bf101b485ed5fd1ef00fd452e306f7/chrome/android/java_sources.gni
[modify] https://crrev.com/67606628f1bf101b485ed5fd1ef00fd452e306f7/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/InnerNodeTest.java
[modify] https://crrev.com/67606628f1bf101b485ed5fd1ef00fd452e306f7/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java
[modify] https://crrev.com/67606628f1bf101b485ed5fd1ef00fd452e306f7/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java

Components: UI>Browser>NewTabPage
Project Member

Comment 57 by bugdroid1@chromium.org, Dec 19 2016

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

commit d3007605eb84788e2d02efc395509ad449f3c035
Author: dgn <dgn@chromium.org>
Date: Mon Dec 19 16:36:01 2016

[NTP Client] Split TreeNode#init into SetParent and SetChildren

Unties Node construction with the initialisation of their parent and
children. This allows controlling when the notifications about child
modifications are propagated, and simplifies the initialisation order.

The InnerNode#setParent() call is now what enables the notifications
and is done as the last step of the NewTabPageAdapter constructor.

ChildNode and InnerNode share almost all of the logic related to
managing the tree and their implementations make use of that without
specific modifications.

This also fixes some bugs related to resetting the section list.

BUG=616090,674023

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

[modify] https://crrev.com/d3007605eb84788e2d02efc395509ad449f3c035/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ActionItem.java
[modify] https://crrev.com/d3007605eb84788e2d02efc395509ad449f3c035/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/AllDismissedItem.java
[modify] https://crrev.com/d3007605eb84788e2d02efc395509ad449f3c035/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ChildNode.java
[modify] https://crrev.com/d3007605eb84788e2d02efc395509ad449f3c035/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/Footer.java
[modify] https://crrev.com/d3007605eb84788e2d02efc395509ad449f3c035/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/InnerNode.java
[modify] https://crrev.com/d3007605eb84788e2d02efc395509ad449f3c035/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/Leaf.java
[modify] https://crrev.com/d3007605eb84788e2d02efc395509ad449f3c035/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
[modify] https://crrev.com/d3007605eb84788e2d02efc395509ad449f3c035/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/OptionalLeaf.java
[modify] https://crrev.com/d3007605eb84788e2d02efc395509ad449f3c035/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ProgressItem.java
[modify] https://crrev.com/d3007605eb84788e2d02efc395509ad449f3c035/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SectionList.java
[modify] https://crrev.com/d3007605eb84788e2d02efc395509ad449f3c035/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SignInPromo.java
[modify] https://crrev.com/d3007605eb84788e2d02efc395509ad449f3c035/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SpacingItem.java
[modify] https://crrev.com/d3007605eb84788e2d02efc395509ad449f3c035/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusItem.java
[modify] https://crrev.com/d3007605eb84788e2d02efc395509ad449f3c035/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java
[modify] https://crrev.com/d3007605eb84788e2d02efc395509ad449f3c035/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/TreeNode.java
[modify] https://crrev.com/d3007605eb84788e2d02efc395509ad449f3c035/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/InnerNodeTest.java
[modify] https://crrev.com/d3007605eb84788e2d02efc395509ad449f3c035/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java

Project Member

Comment 58 by bugdroid1@chromium.org, Dec 21 2016

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

commit a2b8adc6785ae7989af4fbbf7155547c28e90f32
Author: bauerb <bauerb@chromium.org>
Date: Wed Dec 21 14:21:52 2016

[Android NTP] Move dismissing items into the TreeNode interface.

BUG=616090

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

[modify] https://crrev.com/a2b8adc6785ae7989af4fbbf7155547c28e90f32/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
[modify] https://crrev.com/a2b8adc6785ae7989af4fbbf7155547c28e90f32/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ChildNode.java
[modify] https://crrev.com/a2b8adc6785ae7989af4fbbf7155547c28e90f32/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/InnerNode.java
[modify] https://crrev.com/a2b8adc6785ae7989af4fbbf7155547c28e90f32/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/Leaf.java
[modify] https://crrev.com/a2b8adc6785ae7989af4fbbf7155547c28e90f32/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
[modify] https://crrev.com/a2b8adc6785ae7989af4fbbf7155547c28e90f32/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java
[modify] https://crrev.com/a2b8adc6785ae7989af4fbbf7155547c28e90f32/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/OptionalLeaf.java
[modify] https://crrev.com/a2b8adc6785ae7989af4fbbf7155547c28e90f32/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SectionList.java
[modify] https://crrev.com/a2b8adc6785ae7989af4fbbf7155547c28e90f32/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SignInPromo.java
[modify] https://crrev.com/a2b8adc6785ae7989af4fbbf7155547c28e90f32/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java
[modify] https://crrev.com/a2b8adc6785ae7989af4fbbf7155547c28e90f32/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/TreeNode.java
[modify] https://crrev.com/a2b8adc6785ae7989af4fbbf7155547c28e90f32/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerViewTest.java
[modify] https://crrev.com/a2b8adc6785ae7989af4fbbf7155547c28e90f32/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java
[modify] https://crrev.com/a2b8adc6785ae7989af4fbbf7155547c28e90f32/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java

Comment 59 by dgn@chromium.org, Dec 30 2016

I would like to refactor SnippetArticle next... we start having a few places (NewTabPageManager[1], SnippetArticleViewHolder[2], etc) where we check the type of the article using isDownload, isRecentTab, etc. and do different logic afterwards. It sounds like we should have subclasses of SnippetArticle to do implement the different logic. WDYT?

[1]: https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java?q=newtabpagemanager&sq=package:chromium&dr=CSs&l=355
[2]: https://codereview.chromium.org/2602953002/

dgn, what virtual methods do you suggest? Open()/OpenSnippet() seems like a good candidate. Anything else?
Project Member

Comment 62 by bugdroid1@chromium.org, Jan 12 2017

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

commit be82572903bce4c189a2c23548343bd03c3af14a
Author: dgn <dgn@chromium.org>
Date: Thu Jan 12 15:42:07 2017

[NTP Client] Implement offline badge refresh via partial bind

The RecyclerView library has a mechanism to perform partial refresh
of views from the adapter via partial binds[1]. Added support for it
in the NewTabPageAdapter and related classes, and rewrote the refresh
of the offline badge using that.

This patch also fixes an issue where the badge was not removed when an
offline page was deleted

Preview: https://goo.gl/photos/MKD2WXTiNqbQ17xD8

BUG=616090

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

[modify] https://crrev.com/be82572903bce4c189a2c23548343bd03c3af14a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ChildNode.java
[modify] https://crrev.com/be82572903bce4c189a2c23548343bd03c3af14a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/InnerNode.java
[modify] https://crrev.com/be82572903bce4c189a2c23548343bd03c3af14a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/Leaf.java
[modify] https://crrev.com/be82572903bce4c189a2c23548343bd03c3af14a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
[modify] https://crrev.com/be82572903bce4c189a2c23548343bd03c3af14a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NodeParent.java
[modify] https://crrev.com/be82572903bce4c189a2c23548343bd03c3af14a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/OptionalLeaf.java
[modify] https://crrev.com/be82572903bce4c189a2c23548343bd03c3af14a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java
[modify] https://crrev.com/be82572903bce4c189a2c23548343bd03c3af14a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/TreeNode.java
[modify] https://crrev.com/be82572903bce4c189a2c23548343bd03c3af14a/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java
[modify] https://crrev.com/be82572903bce4c189a2c23548343bd03c3af14a/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java
[modify] https://crrev.com/be82572903bce4c189a2c23548343bd03c3af14a/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/InnerNodeTest.java

Project Member

Comment 63 by bugdroid1@chromium.org, Jan 24 2017

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

commit 92bce5eb5a81dd0ed238621622c9ace5e2a36f7c
Author: dgn <dgn@chromium.org>
Date: Tue Jan 24 18:13:20 2017

[NTP Client] Simplify application of partial updates on view holders

Removes the propagation of partial update payload from the tree
interface when binding view holders. Partial updates are now intercepted
by the Adapter and directly applied on the view holder.
This should simplify the partial vs full bind logic.

BUG=616090

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

[modify] https://crrev.com/92bce5eb5a81dd0ed238621622c9ace5e2a36f7c/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/InnerNode.java
[modify] https://crrev.com/92bce5eb5a81dd0ed238621622c9ace5e2a36f7c/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/Leaf.java
[modify] https://crrev.com/92bce5eb5a81dd0ed238621622c9ace5e2a36f7c/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
[modify] https://crrev.com/92bce5eb5a81dd0ed238621622c9ace5e2a36f7c/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/OptionalLeaf.java
[modify] https://crrev.com/92bce5eb5a81dd0ed238621622c9ace5e2a36f7c/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java
[modify] https://crrev.com/92bce5eb5a81dd0ed238621622c9ace5e2a36f7c/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/TreeNode.java
[modify] https://crrev.com/92bce5eb5a81dd0ed238621622c9ace5e2a36f7c/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java
[add] https://crrev.com/92bce5eb5a81dd0ed238621622c9ace5e2a36f7c/chrome/android/java/src/org/chromium/chrome/browser/suggestions/PartialUpdateId.java
[modify] https://crrev.com/92bce5eb5a81dd0ed238621622c9ace5e2a36f7c/chrome/android/java_sources.gni
[modify] https://crrev.com/92bce5eb5a81dd0ed238621622c9ace5e2a36f7c/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/ContentSuggestionsTestUtils.java
[modify] https://crrev.com/92bce5eb5a81dd0ed238621622c9ace5e2a36f7c/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/InnerNodeTest.java

Project Member

Comment 64 by bugdroid1@chromium.org, Jan 26 2017

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

commit 65a19f0cb5910a85ed5cb992fb7c9e0563b33bab
Author: bauerb <bauerb@chromium.org>
Date: Thu Jan 26 16:11:14 2017

[Android NTP] Move more of the dismissal logic into the tree.

Specifically, add a method getItemDismissalGroup() that unifies
specifying both whether an item can be removed and what other items
should be animated along with it (replacing getItemDismissalSibling()).

The UI-specific logic about whether an item can be dismissed (only non-
peeking cards can be dismissed) stays in the ViewHolder classes.

BUG=616090

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

[modify] https://crrev.com/65a19f0cb5910a85ed5cb992fb7c9e0563b33bab/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ActionItem.java
[modify] https://crrev.com/65a19f0cb5910a85ed5cb992fb7c9e0563b33bab/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/CardViewHolder.java
[modify] https://crrev.com/65a19f0cb5910a85ed5cb992fb7c9e0563b33bab/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/InnerNode.java
[modify] https://crrev.com/65a19f0cb5910a85ed5cb992fb7c9e0563b33bab/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/Leaf.java
[modify] https://crrev.com/65a19f0cb5910a85ed5cb992fb7c9e0563b33bab/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
[modify] https://crrev.com/65a19f0cb5910a85ed5cb992fb7c9e0563b33bab/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java
[modify] https://crrev.com/65a19f0cb5910a85ed5cb992fb7c9e0563b33bab/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/OptionalLeaf.java
[modify] https://crrev.com/65a19f0cb5910a85ed5cb992fb7c9e0563b33bab/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SignInPromo.java
[modify] https://crrev.com/65a19f0cb5910a85ed5cb992fb7c9e0563b33bab/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusCardViewHolder.java
[modify] https://crrev.com/65a19f0cb5910a85ed5cb992fb7c9e0563b33bab/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsCategoryInfo.java
[modify] https://crrev.com/65a19f0cb5910a85ed5cb992fb7c9e0563b33bab/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java
[modify] https://crrev.com/65a19f0cb5910a85ed5cb992fb7c9e0563b33bab/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/TreeNode.java
[modify] https://crrev.com/65a19f0cb5910a85ed5cb992fb7c9e0563b33bab/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java
[modify] https://crrev.com/65a19f0cb5910a85ed5cb992fb7c9e0563b33bab/chrome/android/java_sources.gni
[add] https://crrev.com/65a19f0cb5910a85ed5cb992fb7c9e0563b33bab/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsCategoryInfoTest.java
[modify] https://crrev.com/65a19f0cb5910a85ed5cb992fb7c9e0563b33bab/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java

Project Member

Comment 65 by bugdroid1@chromium.org, Jan 30 2017

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

commit 48859482c4d373854a5d4736ab426fb541b99360
Author: bauerb <bauerb@chromium.org>
Date: Mon Jan 30 16:37:06 2017

[Android NTP] Cache number of items under child nodes

For inner nodes this makes getItemCount() more efficient, and for any child node
in general adds assertions that it notifies about the correct number of items
upon modification (which might otherwise only manifest).

BUG=616090

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

[modify] https://crrev.com/48859482c4d373854a5d4736ab426fb541b99360/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ChildNode.java
[modify] https://crrev.com/48859482c4d373854a5d4736ab426fb541b99360/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/InnerNode.java
[modify] https://crrev.com/48859482c4d373854a5d4736ab426fb541b99360/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/Leaf.java
[modify] https://crrev.com/48859482c4d373854a5d4736ab426fb541b99360/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/OptionalLeaf.java
[modify] https://crrev.com/48859482c4d373854a5d4736ab426fb541b99360/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java
[modify] https://crrev.com/48859482c4d373854a5d4736ab426fb541b99360/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/InnerNodeTest.java

Project Member

Comment 66 by bugdroid1@chromium.org, May 25 2017

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

commit afdce664e395ad89ff43e38ffc38eecb6130d374
Author: bauerb <bauerb@chromium.org>
Date: Thu May 25 15:37:18 2017

[Suggestions] Remove TreeNode.getSuggestionAt() in favor of a visitor.

The method is only used in tests, where we can get better coverage by
using the visitor pattern when verifying section contents (as it allows
e.g. also verifying the specific action of an action item).

This also pushes dependencies on domain-specific code out of the
TreeNode interface.

Also implement stringification for (sub-)trees in terms of a visitor.

BUG=616090

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

[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/AboveTheFoldItem.java
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ActionItem.java
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/AllDismissedItem.java
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/CardViewHolder.java
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/Footer.java
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/InnerNode.java
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/Leaf.java
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
[add] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NodeVisitor.java
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/OptionalLeaf.java
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/ProgressItem.java
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SignInPromo.java
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SpacingItem.java
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusItem.java
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/TreeNode.java
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SectionHeader.java
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/java/src/org/chromium/chrome/browser/suggestions/TileGrid.java
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/java_sources.gni
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerViewTest.java
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/InnerNodeTest.java
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java
[modify] https://crrev.com/afdce664e395ad89ff43e38ffc38eecb6130d374/chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/suggestions/ContentSuggestionsTestUtils.java

Owner: ----
Status: Untriaged (was: Started)
Bulk edit: Moving back into the untriaged pool, as I'm leaving the project.

Sign in to add a comment