Hmm...

That map contains SynchronousCompositorHost objects. SCHost is owned by RenderWidgetHostViewAndroid, which should not live past WebContents. Then if you look in aw_contents.h, browser_view_renderer_ is specifically above web_contents_ so former outlives latter.

So yes, I would like an explanation for how this can happen, if the extra RWHVA that's still floating around is from the interstitial or a navigation.

Some updates:

 * This is not specific to multiprocess
 * This repros when we call WebView.destroy() on a webview with visible interstitials (this is because interstitials teardown asynchronously [1])
 * This can crash in production too: when this bug repros, the interstitial's SCHost outlives its SCClient. When the SCHost is destroyed, it tries to notify its freed SCClient
 * I see a microdump when I try to repro on production builds. Regression range appears to be 67.0.3370.0..67.0.3371.0. But this situation is undefined behavior, so it's very possible this has always been broken and we were just lucky that it didn't crash before. For the record, the microdump I see matches crashes with this stack [2]

Potential fix is at http://crrev/c/1045271.

[1] https://cs.chromium.org/chromium/src/content/browser/frame_host/interstitial_page_impl.cc?l=295&rcl=b0a590014db6c49b45294dc1fcc8013424896eb9
[2] https://crash.corp.google.com/browse?q=product.name%3D%27AndroidWebView%27+AND+stable_signature+LIKE+%27ui%3A%3ADelegatedFrameHostAndroid%3A%3ADetachFromCompositor%25%27

Renaming this, because this is UAF after passing through the DCHECK. Adding RBS because this appears to cause real crashes, and seems like a kind-of-likely scenario.

ntfschr@,
Is there anyway we can replicate this manually? If so, could u please provide us with steps? In that way, we can test and verify fix, when it is merged. 

Thanks!

Yup. I've attached a repro APK. Steps:

0. Install that APK
1. Start the app, observe a safe browsing interstitial. Do not click the interstitial
2. Click the button on the bottom of the screen, which says "destroy webview"
3. Observe the app crashes

Expected results:

3. The webview section of the app should appear blank white. The app should *not* crash.

Please let me know ASAP if you can't repro the issue.

Deleted: app-debug.apk 1.5 MB

app-debug.apk 1.5 MB Download

This issue is reproducible on Nexus O/Pixel 2XL devices, but not on Samsung devices 
___________________
I'm not able to repro this issue on 
Verizon/heroqltevzw/heroqltevzw:7.0/NRD90M/G930VVRU4BRC2:user/release-keys with M67 webview, First when i launch the app > it shows white screen with (walware page Example, rather then the red screen), and when i clicked on Destroy webview button > the page is white (does not crash)

______________
Pixel 2 XL/O - repro (crashes)
Pixel XL/O - repro (crashes)
Samsung S7/N - does not repro (shows white screen, don't crash)
Samsung S6/rooted/L - does not repro (shows white screen, don't crash)
_________________

the webview build for comment#6 is 67.0.3396.43

Nate are you looking into this issue?

Yes, still investigating this.

Any update here?

I have chatted offline with ntfschr@, he is going to work in this fix today.

Thanks!

Update: still investigating test failures, but hoping that patchset#2 is an improvement.

Update: patchset#3 should fix more of the test failures.

The root problem in my initial patchset is that interstitial destruction is very messy. An interstitial might be asked twice to destroy itself: once when the user navigates away from the page and once when the webcontents is destroyed.

Update: boliu@ reviewed the CL. After the CL lands on trunk, I'll provide a cost/benefit analysis of a merge.

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

commit cda8254ef3510bc369c64ffb7989d29ab044e987
Author: Nate Fischer <ntfschr@chromium.org>
Date: Wed May 23 05:04:21 2018

AW: fix crash from destroying WebView with interstitials

Previously, destroy()'ing a WebView which had visible interstitials
would violate a DCHECK (or, crash at a later point if DCHECKs are
disabled). The interstitial would clean up asynchronously, after the
corresponding WebContents was destroyed. This violated a DCHECK in
BrowserViewRenderer's destructor, because it still had a pointer to the
interstitial's SynchronousCompositorHost.

This modifies RenderWidgetHostViewAndroid to cleanup these references to
the interstitial compositor synchronously while the WebContents is being
destroyed. The rest of the interstitial teardown will continue to be
asynchronous.

Bug:  837829 
Test: run_webview_instrumentation_test_apk -f SafeBrowsingTest#testDestroyWebViewWithInterstitialShowing
Change-Id: Ie63fcde827aee801b9e7fc1e4b62a43677133a33
Reviewed-on: https://chromium-review.googlesource.com/1045271
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560949}
[modify] https://crrev.com/cda8254ef3510bc369c64ffb7989d29ab044e987/android_webview/javatests/src/org/chromium/android_webview/test/SafeBrowsingTest.java
[modify] https://crrev.com/cda8254ef3510bc369c64ffb7989d29ab044e987/content/browser/frame_host/interstitial_page_impl.cc
[modify] https://crrev.com/cda8254ef3510bc369c64ffb7989d29ab044e987/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/cda8254ef3510bc369c64ffb7989d29ab044e987/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/cda8254ef3510bc369c64ffb7989d29ab044e987/content/browser/renderer_host/render_widget_host_view_base.h

I'm at the MTV Chrome offsite today and tomorrow, but I'll request a merge now and cherry-pick (if approved) on Thursday afternoon.

Impact:

 - this bug involves undefined behavior. So it may or may not crash (it didn't crash on the samsung devices we tried in comment #6). But this bug is not "device specific," it happen reliably, even if side effects are not always fatal.
 - I first noticed the crash in M67, hence the request to merge
 - This happens whenever we destroy natives with an interstitial showing. One possible scenario would be if a user is browsing and sees an interstitial, and closes the webview to go back to the main app. I don't have numbers for how often this exact scenario occurs.
 - Without this change, users apps will (for no user-apparent reason) crash
 - Only impacts the case where Safe Browsing is triggered, which is relatively rare

Risk:

 - The bulk of this change only impacts WebView. I believe clank is not affected.

This bug requires manual review: We are only 5 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

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

Ping!

Merging now

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

commit b17bd28c3ed771d712314f0d6da56ff9a1554719
Author: Nate Fischer <ntfschr@chromium.org>
Date: Thu May 24 23:27:55 2018

AW: fix crash from destroying WebView with interstitials

Previously, destroy()'ing a WebView which had visible interstitials
would violate a DCHECK (or, crash at a later point if DCHECKs are
disabled). The interstitial would clean up asynchronously, after the
corresponding WebContents was destroyed. This violated a DCHECK in
BrowserViewRenderer's destructor, because it still had a pointer to the
interstitial's SynchronousCompositorHost.

This modifies RenderWidgetHostViewAndroid to cleanup these references to
the interstitial compositor synchronously while the WebContents is being
destroyed. The rest of the interstitial teardown will continue to be
asynchronous.

Bug:  837829 
Test: run_webview_instrumentation_test_apk -f SafeBrowsingTest#testDestroyWebViewWithInterstitialShowing
Change-Id: Ie63fcde827aee801b9e7fc1e4b62a43677133a33
Reviewed-on: https://chromium-review.googlesource.com/1045271
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#560949}(cherry picked from commit cda8254ef3510bc369c64ffb7989d29ab044e987)
Reviewed-on: https://chromium-review.googlesource.com/1072709
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#698}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/b17bd28c3ed771d712314f0d6da56ff9a1554719/android_webview/javatests/src/org/chromium/android_webview/test/SafeBrowsingTest.java
[modify] https://crrev.com/b17bd28c3ed771d712314f0d6da56ff9a1554719/content/browser/frame_host/interstitial_page_impl.cc
[modify] https://crrev.com/b17bd28c3ed771d712314f0d6da56ff9a1554719/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/b17bd28c3ed771d712314f0d6da56ff9a1554719/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/b17bd28c3ed771d712314f0d6da56ff9a1554719/content/browser/renderer_host/render_widget_host_view_base.h

Verified on Pixel2/OPM1.171019.028 having 68.0.3440.7 /69.0.3444.0 

Verified with verification steps mentioned in comment #5 on Samusng S8/R16NW having 67.0.3396.68