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

Issue 664319 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Inbox is showing spinner, and emails take a long time to load

Project Member Reported by sgu...@chromium.org, Nov 10 2016

Issue description

Grace has shown me that on her show, in a long thread of emails, Inbox is showing spinner rather than loading the emails. It seems like the spinner is running for at least 15-20 seconds (perhaps more). 

This happens on canary and dev. It is not clear if it happens on beta. Not happening on stable.

can test team repro the problem and bisect please? Grace has the email with the problem and seems very easy to repro. 
 

Comment 1 by sgu...@chromium.org, Nov 10 2016

Status: Assigned (was: Untriaged)

Comment 2 by sgu...@chromium.org, Nov 10 2016

Cc: klo...@chromium.org
sgurun@ - We are looking into it.
Thanks!
Labels: Needs-Bisect
Owner: sbash...@chromium.org
Reproducible on Lastest Chrome dev (56.0.2913.4) and Chrome Canary(56.0.2915.0) .
Not Reproducible in Chrome Stable  . Attaching Logcat and Video in 
 http://go/chrome-androidlogs1/6/664319
Bisecting in progress....
Bisect Range :  https://chromium.googlesource.com/chromium/src/+log/56.0.2906.3..56.0.2913.4?pretty=fuller&n=10000

56.0.2906.3 - Good build
... ( 11 builds)
56.0.2913.4 - Bad Build 
Currently experiencing Server error due to https://screenshot.googleplex.com/QjiiOs4XCqp  and hence will provide more info  on the first bad build asap.
Labels: -Type-Bug -Needs-Bisect Type-Bug-Regression
Bisect Range : https://chromium.googlesource.com/chromium/src/+log/56.0.2907.0..56.0.2908.0?pretty=fuller&n=10000

Good build: 56.0.2907.0 
Bad build: 56.0.2908.0

Steps to reproduce:
(1)Launch Inbox,open any mail which contains a replied message
(2)Observe

Devices tested:
Gionee F103/LRX21M, HTC Desire 630/MMB29M, Pixel XL/NDE63H,Nexus
5X/NRD90R

Please find the logcat & screenrecord @ http://go/chrome-androidlogs1/6/664319-1

Comment 8 by sgu...@chromium.org, Nov 11 2016

Labels: -M-55 -ReleaseBlock-Stable M-56 ReleaseBlock-Beta

Comment 9 by sgu...@chromium.org, Nov 11 2016

Owner: ----
Status: Available (was: Assigned)
in this range, the only change that touches android_webview is the revert of async draw. I don't know if something else changed in the code that a revert would cause such a regression. However, Bo re-enabled async draw yesterday.

Unfortunately I have no other candidate that I can put the blame on. will check today.
One more observation on Gmail :
Observing loading Spinner in Tapping mails while testing Alpha on Nexus6/NMF26I,Samsung J5/MMB29M,Nexus 9 / M0B31O having  53.0.2785.124, 54.0.2480.85, 56.0.2913.5 looks like a app issue . 
we already have a spinner related bug, due to a recent change, but the range is different. crbug/567044. The range above could be incorrect?
but that bug above seems to be in M54, maybe not.
Owner: scottmg@chromium.org
This is caused by https://codereview.chromium.org/2381503003. Reverting fixes it. Note that even though the CL title sounds like it is a test fix, it actually changes the code. 
Status: Started (was: Available)
Thanks, revert is in CQ. That CL only shipped in 56, so I don't think any merge of the revert will be required.
Project Member

Comment 15 by bugdroid1@chromium.org, Nov 14 2016

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

commit fe5ea3f7cb872ea6fa2e560cfefc9185393a8cb2
Author: scottmg <scottmg@chromium.org>
Date: Mon Nov 14 19:19:49 2016

Revert of PlzNav: Fix NavigationControllerBrowserTest.EnsureSamePageNavigationUpdatesFrameNaviga… (patchset #21 id:500001 of https://codereview.chromium.org/2381503003/ )

Reason for revert:
 https://crbug.com/664319 .

Original issue's description:
> PlzNav: Fix NavigationControllerBrowserTest.EnsureSamePageNavigationUpdatesFrameNaviga…
>
> This test is failing in PlzNav because
> https://cs.chromium.org/chromium/src/content/browser/frame_host/navigation_controller_impl_browsertest.cc?q=%22Simulate+the+user+hitting+Enter%22&sq=package:chromium&l=5943&dr=C
> is creating a new navigation entry, so the GoBack() goes to the wrong
> place.
>
> It's doing that because
> FrameHostMsg_DidCommitProvisionalLoad_Params.did_create_new_entry is
> true.
>
> That in turn is because SendDidCommitProvisionalLoad is getting a commit
> type of WebStandardCommit (not WebHistoryInertCommit like it does in
> non-PlzNav).
>
> *That* is because here
> https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/loader/FrameLoader.cpp?rcl=0&l=839
> the url being checked is the already-redirected one in the PlzNavigate
> case (sub.a.com), but m_documentLoader->urlForHistory() is the
> pre-redirected one (a.com/server-redirect?sub...).
>
> So, this works in non-PlzNav because Blink tells us that it should
> replace the current entry (i.e. it's like a reload).
>
> Instead of relying on Blink to determine that, check if the render frame
> node's last committed url is the original url we navigated to there, and
> that we're not just mucking with history navigation, in which case we
> convert the normal load to a reload.
>
> R=nasko@chromium.org,clamy@chromium.org
> BUG=630103, 475027 , 536102
>
> Committed: https://crrev.com/28bbbb16194c7337e22c3e0bda752ae6b4b681cf
> Cr-Commit-Position: refs/heads/master@{#429302}

TBR=nasko@chromium.org,clamy@chromium.org,creis@chromium.org,boliu@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=630103, 475027 , 536102, 664319 

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

[modify] https://crrev.com/fe5ea3f7cb872ea6fa2e560cfefc9185393a8cb2/content/browser/frame_host/navigation_controller_impl.cc
[modify] https://crrev.com/fe5ea3f7cb872ea6fa2e560cfefc9185393a8cb2/testing/buildbot/filters/browser-side-navigation.linux.content_browsertests.filter

Comment 16 by boliu@chromium.org, Nov 15 2016

 Issue 665469  has been merged into this issue.
Project Member

Comment 17 by bugdroid1@chromium.org, Nov 15 2016

Labels: merge-merged-2919
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1531722f6aa1faa33de433331927a4fc24a4a197

commit 1531722f6aa1faa33de433331927a4fc24a4a197
Author: Alex Mineer <amineer@chromium.org>
Date: Tue Nov 15 17:08:02 2016

Revert "PlzNav: Fix NavigationControllerBrowserTest.EnsureSamePageNavigationUpdatesFrameNaviga…"

This reverts commit 28bbbb16194c7337e22c3e0bda752ae6b4b681cf.

Reverting on branch 2919 to ship a dev release.

BUG= 664319 

Cr-Commit-Position: refs/branch-heads/2919@{#3}
Cr-Branched-From: 9fd27ddeb13974b24cfa42261b940f8de378117a-refs/heads/master@{#431801}

[modify] https://crrev.com/1531722f6aa1faa33de433331927a4fc24a4a197/content/browser/frame_host/navigation_controller_impl.cc
[modify] https://crrev.com/1531722f6aa1faa33de433331927a4fc24a4a197/testing/buildbot/filters/browser-side-navigation.linux.content_browsertests.filter

Issue not reproducible anymore, tested on Galaxy S5 (arm_32/SM-G900H) / MMB29K, Pixel C/ N2F46, Galaxy S7 (SM-G930F) / MMB29M, Acer Predator/LMY47I, Webview: 56.0.2919.3.
Status: Fixed (was: Started)
Based on comments above, this should be fixed across the board.
Status: Verified (was: Fixed)
Verified in Nexus 9 (MOB31U) having  56.0.2924.10 and Spinner no longer reproducible.
The patch is here  https://codereview.chromium.org/2580753002
Cc: ananta@chromium.org
TE aluo@ built locally and provided the apk and still able to repro the Spinner issue on Nexus 6/ N2F76 .
Project Member

Comment 23 by bugdroid1@chromium.org, Dec 22 2016

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

commit 3bdd8aea0439f2c6a50d24c3c805bc7e579d5a2c
Author: ananta <ananta@chromium.org>
Date: Thu Dec 22 17:11:55 2016

Ensure that duplicate navigations are tagged as reload in the browser.
This includes the cases where we are navigating to a URL which was
committed previously or to a URL for which we haven't received a commit.

scottmg's patch for reference is here https://codereview.chromium.org/2381503003. This was reverted due to failures on Android.

The failure was seen in cases like this. Navigation to url1->url2->url1.
If the first navigation is not committed yet, then we would treat the last
navigation to url1 as a reload. This causes inbox on Android to hang.

The proposed fix is to use the last pending navigation if it exists or
the last committed navigation to compare whether the current navigation is
a reload. This fixes the issues with Android

Changes in this patch are as below:
1. Add members in the NavigationControllerImpl class to hold the last
   pending navigation entry, its index, and the last transient index.
   The last transient index is used to avoid treating navigations as
   reloads if we have an interstitial being displayed.

2. Add a flag in the NavigationEntryImpl class to indicate whether there
   was a SSL error in the navigation. This was added to fix browser test
   failures. We avoid treating navigations as reloads when the last
    pending navigation has a SSL error.

3. We compare the url of the last pending or committed entry with the
   current navigation entry to see if it is a reload. There are a number
   of cases to consider here. Please look at the code.

4. Added a unittest NavigationControllerTest.MultipleNavigationsAndReload
   which tests various combinations of pending and committed navigations
   to ensure that the navigation is tagged correctly as reload or not.

The non plznavigate case does not have the bug when the EnsureSamePageNavigationUpdatesFrameNavigationEntry test because Blink tells us the correct navigation history, i.e. whether a navigation is a
reload, etc.

R=nasko@chromium.org,clamy@chromium.org
BUG=630103,  475027 , 536102,  664319 , 676235
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/3bdd8aea0439f2c6a50d24c3c805bc7e579d5a2c/content/browser/frame_host/navigation_controller_impl.cc
[modify] https://crrev.com/3bdd8aea0439f2c6a50d24c3c805bc7e579d5a2c/content/browser/frame_host/navigation_controller_impl.h
[modify] https://crrev.com/3bdd8aea0439f2c6a50d24c3c805bc7e579d5a2c/content/browser/frame_host/navigation_controller_impl_unittest.cc
[modify] https://crrev.com/3bdd8aea0439f2c6a50d24c3c805bc7e579d5a2c/content/browser/frame_host/navigation_entry_impl.cc
[modify] https://crrev.com/3bdd8aea0439f2c6a50d24c3c805bc7e579d5a2c/content/browser/frame_host/navigation_entry_impl.h
[modify] https://crrev.com/3bdd8aea0439f2c6a50d24c3c805bc7e579d5a2c/content/browser/ssl/ssl_manager.cc

Sign in to add a comment