Issue metadata
Sign in to add a comment
|
Inbox is showing spinner, and emails take a long time to load |
||||||||||||||||||||||
Issue descriptionGrace 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.
,
Nov 10 2016
,
Nov 10 2016
sgurun@ - We are looking into it. Thanks!
,
Nov 10 2016
,
Nov 10 2016
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....
,
Nov 11 2016
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.
,
Nov 11 2016
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
,
Nov 11 2016
,
Nov 11 2016
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.
,
Nov 12 2016
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 .
,
Nov 12 2016
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?
,
Nov 12 2016
but that bug above seems to be in M54, maybe not.
,
Nov 12 2016
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.
,
Nov 14 2016
Thanks, revert is in CQ. That CL only shipped in 56, so I don't think any merge of the revert will be required.
,
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
,
Nov 15 2016
Issue 665469 has been merged into this issue.
,
Nov 15 2016
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
,
Nov 15 2016
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.
,
Nov 29 2016
Based on comments above, this should be fixed across the board.
,
Nov 29 2016
Verified in Nexus 9 (MOB31U) having 56.0.2924.10 and Spinner no longer reproducible.
,
Dec 15 2016
The patch is here https://codereview.chromium.org/2580753002
,
Dec 16 2016
TE aluo@ built locally and provided the apk and still able to repro the Spinner issue on Nexus 6/ N2F76 .
,
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 |
|||||||||||||||||||||||
Comment 1 by sgu...@chromium.org
, Nov 10 2016