Issue metadata
Sign in to add a comment
|
Clank sad tab/aw snap |
||||||||||||||||||||||
Issue descriptionChrome Version: 69.0.3475.0 OS: Android 8.1.0 (on Pixel 2 XL) What steps will reproduce the problem? (1) Visit http://jalopnik.com (2) Click any story (3) Press back arrow (the Android system one) What is the expected result? It goes back to the jalopnik.com main page What happens instead? I get a sad tab. If I click 'reload' on the sad tab screen, the main page reloads OK. I can't repro on other Chrome versions on my phone (eg stable), so this may somehow be specific to me or my profile? It's 100% reproducible (across phone reboots and OS upgrades) on the version is does happen on though. Looking in about:crashes, I don't see any crash recorded for this. This has been happening for a while now.
,
Jul 3
Tested the issue in Android and able to reproduce the issue. Steps Followed: 1. Launch Chrome. 2. Visit http://jalopnik.com 3. Click any story 4. Press back arrow (the Android system one) and Observed tab crash Chrome versions tested: 69.0.3475.0(Dev), 69.0.3479.2(Latest canary) OS: Android 8.1.0 Android Devices: Pixel 2 XL Using the per-revision bisect providing the bisect results, Good Build - 69.0.3447.0 563479 Bad Build - 69.0.3448.0 563958 You are looking for a change made after 563609(GOOD), but before 563610(BAD). CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+/6b92fcfb1cf296130f6cb38213aa52a0324c9e46 @ clamy : Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to owner concerned. Please navigate to below link for log's -- go/chrome-androidlogs/859386 Thanks!
,
Jul 3
Thanks for bisecting this. Any idea why this was not showing up in chrome:crashes? I'm worried that this means this (or other crashes) could be widespread in the wild but we are getting no crash info about it.
,
Jul 7
Happens in chromeOS too. Version: Chrome/69.0.3473.0 Platform: 10820.0.0 (Official Build) dev-channel gandof Firmware: Google_Gandof.6301.155.9
,
Jul 8
Adding ChromeOS per comment #4. This looks to be two separate problems - the crash itself, but also that it doesn't show up in about:crashes. I'll leave this bug to be specific to the crash, and file a new one about it not appearing in about:crashes.
,
Jul 8
Filed bug #861607 to track the 'not showing in chrome:crashes' problem.
,
Jul 10
I can't repro on trunk using chrome_public_apk, nor on the latest Canary (69.0.3487.0). If you're still seeing the issue, could you provide a crash stack? Thanks!
,
Jul 10
Going to close this out for now; WontFix, cannot reproduce. If the issue continues, please reopen. Thanks!
,
Jul 11
Wait a sec, this was reproed by at least three people (me, QA, and an external contributor), across two OSs. And we have a bisect down the suspect CL. Please don't WontFix one hour after a request by the engineer for more information. @clamy - this is not uploading a crash stack on my phone. @jbanavatu - can you help with crash stack information? I can't read the logs link you posted earlier.
,
Jul 11
@mikelawther: can you still reproduce the issue on the latest Canary? The suspected CL is not easily revertible, and there's been a lot of work going on in the area it touches so the issue might be gone.
,
Jul 11
FWIW I saw this bug using canary myself a couple weeks back, but it went away for me there as well shortly afterwards, so I'm working under the assumption the issue is fixed. I also confirmed that I can repro 100% in dev, but not in latest canary using same steps. So, I'd tend to concur with clamy@ re: this likely having been addressed via another patch. We could potentially do a reverse bisect to find which CL fixed the issue if we wanted explicit confirmation of which patch fixed this, but I'm unsure if it's worth the time personally, kind of leave that to clamy@ since they'll know best what work has been ongoing in this area.
,
Jul 11
Thanks for the additional details, Alex. I recommend we remove RBD at this time and proceed with releasing Dev. I recommend updating to RBS for now so we do not lose track of it. If it continues after releasing a new version to dev, we can update to RBD and hope to increase urgency.
,
Jul 11
,
Jul 11
With DCHECKs enabled, it hit this DCHECK in generated mojo code:
DCHECK(!connected)
<< "FrameNavigationControl::CommitNavigationCallback was destroyed without "
<< "first either being run or its corresponding binding being closed. "
<< "It is an error to drop response callbacks which still correspond "
<< "to an open interface pipe.";
https://cs.chromium.org/chromium/src/out/Debug/gen/content/common/frame.mojom.cc?rcl=eb2e049e1b98cf00c75d2540b3fb4c32b4d5e6d6&l=250
+rockot for mojo help. So what happens without DCHECKs?
Also I could only reproduce this with site isolation disabled, although not sure if that's important
,
Jul 11
rockot: does that just generate a mojo channel error on the browser side if the DCHECk doesn't fire? that would explain the other behavior of not generating a crash report from this, because the renderer process never actually crashed
,
Jul 11
This means that something is never getting to get a reply to its call to FrameNavigationControl.CommitNavigation[1]. In practice there are quite often no harmful side effects to this because the code responsible for responding will imminently close the binding anyway. The fix is typically just to make sure that in such cases, the binding is closed before uncalled response callbacks are discarded.
,
Jul 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/138153b186554c2077e867efbeb71e0342b70159 commit 138153b186554c2077e867efbeb71e0342b70159 Author: Ken Rockot <rockot@chromium.org> Date: Fri Jul 13 23:31:57 2018 IPC: Avoid closing Channel endpoints on error We never want to explicitly close a Channel endpoint until after the corresponding child process is dead. This prevents that from happening in some cases where it's still possible today. TBR=dcheng@chromium.org Bug: 859386 ,861607, 863430 Change-Id: I2858b6e011e06a000ae976c1e6ae96595cf67a85 Reviewed-on: https://chromium-review.googlesource.com/1135712 Commit-Queue: Ken Rockot <rockot@chromium.org> Reviewed-by: Ken Rockot <rockot@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Cr-Commit-Position: refs/heads/master@{#575115} [modify] https://crrev.com/138153b186554c2077e867efbeb71e0342b70159/ipc/DEPS [modify] https://crrev.com/138153b186554c2077e867efbeb71e0342b70159/ipc/ipc_channel_mojo_unittest.cc [modify] https://crrev.com/138153b186554c2077e867efbeb71e0342b70159/ipc/ipc_mojo_bootstrap.cc [modify] https://crrev.com/138153b186554c2077e867efbeb71e0342b70159/ipc/ipc_test.mojom
,
Jul 16
@boliu, @rockot - THANK YOU for persisting and investigating this! Looks like a genuine bug was fixed here.
,
Jul 20
Verified fix in 69.0.3497.2 / Pixel 2 XL / 8.1.0. - No Aw,snap on 'back' Is there anymore work pending here, or the issue can be closed?
,
Jul 23
I don't think there's anything pending, closing this issue.
,
Jul 23
[Auto-generated comment by a script] We noticed that this issue is targeted for M-69; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-69 label, otherwise remove Merge-TBD label. Thanks.
,
Jul 23
Fix was verified in 69.0.3497.2 so no need for merge.
,
Jul 24
Just to follow up - this issue may have been responsible for ~25% of sad tabs (where the user clicked reload on a sad tab) on Android dev channel (https://uma.googleplex.com/p/chrome/timeline_v2/?sid=f0e14b372b4723cb10fd4ff3b0225875) In the release after the bisected CL above landed, sad-tab-reload events on Android spiked by ~33%. And then after the fix CL landed, they dropped again. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by jbanavatu@chromium.org
, Jul 2