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

Issue 626831 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Not on Chrome anymore
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Suggestions spontaneously cleared while they are not expired

Project Member Reported by dgn@chromium.org, Jul 8 2016

Issue description

1. Open Chrome on NTP, see snippets loaded from a previous session
2. Wait... wait...
3. Observe snippets be cleared even though they are not expired.


D/cr_Ntp  : [NewTabPageAdapter.java:140] Received 7 new snippets.
D/cr_NtpCards: [StatusListItem.java:113] Registering card for status: History Sync Disabled
D/cr_Ntp  : [NewTabPageAdapter.java:140] Received 2 new snippets.
D/cr_NtpCards: [StatusListItem.java:70] Registering card for status: No Snippets
W/cr_NtpCards: The RecyclerView items are not attached, can't determine the content height: snap=null, last=null. Using full height: 2105 


Might be related to the temporary status being wrong. In that case properly handling issue 624338 should fix it. Better investigate soon though.
 
demo.mp4
3.6 MB View Download

Comment 1 by nepper@chromium.org, Jul 19 2016

Cc: bauerb@chromium.org
Labels: -Type-Bug zine-triaged M-54 M-53 Type-Bug-Regression
This is happening a lot to me. This may actually block dogfood as well if it happens consistently on M53.
Project Member

Comment 2 by sheriffbot@chromium.org, Jul 19 2016

Labels: -M-53 MovedFrom-53
Moving this nonessential bug to the next milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 3 by dgn@chromium.org, Jul 20 2016

Labels: -zine-16-07-11 zine-16-07-18
Status: Started (was: Assigned)

Comment 4 by fi...@chromium.org, Jul 20 2016

I can reproduce this behavior 100% if I kill Chrome via the app switcher. 

@dgn: Are you sure that this could be related to issue 624338 ? It is about the reload-spinner.


Comment 5 by dgn@chromium.org, Jul 20 2016

Fix for this is in review: https://codereview.chromium.org/2166753003/

The spinner is another issue, yes, but it will be needed for when we don't have snippets in the DB. We become aware of history sync being enabled very late and we don't have a good way to handle that in the UI for now ( issue 627488 )

Project Member

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

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

commit fc4b68d76ae1bc8db479c14be1b306e9185a1626
Author: dgn <dgn@chromium.org>
Date: Thu Jul 21 14:06:11 2016

[NTP Client] Stop clearing suggestions to show the RELOAD card.

We used to clear already loaded suggestions unconditionally when
receiving notifications of the service status having changed. During
startup we used to pull suggestions right away and get the status OK
notification afterwards, which cleared them. We now properly detect
and do nothing in that case.

BUG= 626831 

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

[modify] https://crrev.com/fc4b68d76ae1bc8db479c14be1b306e9185a1626/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
[modify] https://crrev.com/fc4b68d76ae1bc8db479c14be1b306e9185a1626/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java

Comment 7 by dgn@chromium.org, Jul 21 2016

Status: Fixed (was: Started)

Comment 8 by dgn@chromium.org, Jul 21 2016

Labels: Merge-Request-53
Status: Started (was: Fixed)
Reopening to request M53 merge.

Comment 9 by dimu@google.com, Jul 21 2016

Labels: -Merge-Request-53 Merge-Approved-53 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M53 (branch: 2785)

Comment 10 by dgn@chromium.org, Jul 21 2016

cherry-pick fails with https://codereview.chromium.org/2162093002 (SnippetArticle -> SnippetArticleListItem) missing, and cherry-picking that one fails on https://codereview.chromium.org/2122993003 (remove unused Interests code).

I might just merge with the old name instead or adding the missing CLs.
Project Member

Comment 11 by bugdroid1@chromium.org, Jul 22 2016

Labels: -merge-approved-53 merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/86cb36f424317ea3128f4751f052d2a5aeb9e51e

commit 86cb36f424317ea3128f4751f052d2a5aeb9e51e
Author: dgn <dgn@chromium.org>
Date: Fri Jul 22 10:13:53 2016

[M53][NTP Client] Stop clearing suggestions to show the RELOAD card.

We used to clear already loaded suggestions unconditionally when
receiving notifications of the service status having changed. During
startup we used to pull suggestions right away and get the status OK
notification afterwards, which cleared them. We now properly detect
and do nothing in that case.

BUG= 626831 
TBR=bauerb@chromium.org
NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2166753003
Cr-Commit-Position: refs/heads/master@{#406837}
(cherry picked from commit fc4b68d76ae1bc8db479c14be1b306e9185a1626)

Review-Url: https://codereview.chromium.org/2169393002
Cr-Commit-Position: refs/branch-heads/2785@{#293}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/86cb36f424317ea3128f4751f052d2a5aeb9e51e/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
[modify] https://crrev.com/86cb36f424317ea3128f4751f052d2a5aeb9e51e/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java

Comment 12 by dgn@chromium.org, Jul 22 2016

Status: Fixed (was: Started)

Comment 13 by dgn@chromium.org, Aug 4 2016

Made a request to merge https://codereview.chromium.org/2216683002/ in M53, approved by kerz@ on the CL.
Project Member

Comment 14 by bugdroid1@chromium.org, Aug 4 2016

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

commit beb43eea08f42bef2ff83d72c4d7a51cd42fb3eb
Author: dgn <dgn@chromium.org>
Date: Thu Aug 04 17:43:45 2016

[M53][NTP Client] Fix test compilation on branch

Fix the inconsistent class name usage introduced by
https://codereview.chromium.org/2166753003

BUG= 626831 
NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2216683002
Cr-Commit-Position: refs/branch-heads/2785@{#504}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/beb43eea08f42bef2ff83d72c4d7a51cd42fb3eb/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java

Sign in to add a comment