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

Issue 906340 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android
Pri: 1
Type: Bug

Blocking:
issue 894899



Sign in to add a comment

CHECK failure: GetFrameWidget() in render_widget.cc

Project Member Reported by ClusterFuzz, Nov 17

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=6524978859868160

Fuzzer: puzzor
Job Type: linux_asan_chrome_v8_arm
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  GetFrameWidget() in render_widget.cc
  content::RenderWidget::RequestNewLayerTreeFrameSink
  content::RenderWidget::RequestNewLayerTreeFrameSink
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_v8_arm&range=607784:607802

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6524978859868160

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Project Member

Comment 1 by ClusterFuzz, Nov 17

Components: Internals>Compositing Internals>Core
Labels: Test-Predator-Auto-Components
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Project Member

Comment 2 by ClusterFuzz, Nov 17

Labels: Test-Predator-Auto-Owner
Owner: danakj@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/73063d16b0fb9605e224e0d35f516e3b7d741d1e (Add CHECKs to track down why we're making frame sinks when we shouldn't).

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.
Blocking: 894899
Issue 907104 has been merged into this issue.
Cc: a...@chromium.org enne@chromium.org ajwong@chromium.org dcheng@chromium.org piman@chromium.org
Clusterfuzz reproduce tool no luck for me on this one
Oh ya remembering context now, this is almost certainly from Send(new FrameMsg_Delete(routing_id_)); in ~RenderFrameHostImpl() which dcheng@ is working to remove. Bug TBD
If we don't resolve that bug by branch point I will solve this (and https://bugs.chromium.org/p/chromium/issues/detail?id=896836) temporarily by just not grabbing a URL in this scenario when making the frame sink. It'll be a wasted frame sink, for a compositor that thinks its visible but isn't. But it won't crash.
Project Member

Comment 9 by ClusterFuzz, Dec 2

Labels: OS-Android
Project Member

Comment 10 by bugdroid1@chromium.org, Dec 5

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

commit be7cebf02ef4a1cfc740a3ff30db71597772aba2
Author: danakj <danakj@chromium.org>
Date: Wed Dec 05 00:20:39 2018

For M72 branch, avoid crashing when there's no main frame around.

Sometimes we get a request for a compositor frame sink when the main
frame is not present and this would crash. Remove the debugging CHECK()
and handle the case for merging to the M72 branch. Will revert again
back on trunk, as dcheng@ works toward removing the one case we believe
can cause this.

R=piman@chromium.org

Change-Id: I89686a3c2f83156f72e77940099be56bcc401d57
Bug: 906340
Reviewed-on: https://chromium-review.googlesource.com/c/1362004
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613780}
[modify] https://crrev.com/be7cebf02ef4a1cfc740a3ff30db71597772aba2/content/renderer/render_widget.cc

Labels: Merge-Request-72
Project Member

Comment 12 by sheriffbot@chromium.org, Dec 6

Labels: -Merge-Request-72 Hotlist-Merge-Approved Merge-Approved-72
Your change meets the bar and is auto-approved for M72. Please go ahead and merge the CL to branch 3626 manually. Please contact milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Pls merge your change to M72 branch 3626 ASAP so we can pick it up for next  Dev & Beta release. Thank you.
Project Member

Comment 14 by bugdroid1@chromium.org, Dec 7

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b18768f234966f5aca498cae15c7f405d273486a

commit b18768f234966f5aca498cae15c7f405d273486a
Author: danakj <danakj@chromium.org>
Date: Fri Dec 07 17:01:11 2018

For M72 branch, avoid crashing when there's no main frame around.

Sometimes we get a request for a compositor frame sink when the main
frame is not present and this would crash. Remove the debugging CHECK()
and handle the case for merging to the M72 branch. Will revert again
back on trunk, as dcheng@ works toward removing the one case we believe
can cause this.

R=piman@chromium.org
TBR=danakj@chromium.org

(cherry picked from commit be7cebf02ef4a1cfc740a3ff30db71597772aba2)

Change-Id: I89686a3c2f83156f72e77940099be56bcc401d57
Bug: 906340
Reviewed-on: https://chromium-review.googlesource.com/c/1362004
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613780}
Reviewed-on: https://chromium-review.googlesource.com/c/1368025
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#135}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/b18768f234966f5aca498cae15c7f405d273486a/content/renderer/render_widget.cc

Comment 15 Deleted

Once this is resolved we can
1. Remove the conditional check to avoid grabbing a URL in DoRequestNewLayerTreeFrameSink
2. Enforce that RenderWidget::GetFrameWidget() never returns null, by moving early outs to IPC handler entry points.
Project Member

Comment 17 by bugdroid1@chromium.org, Dec 13

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

commit f51754379ee4b2d22db968d0c8ea21754b300246
Author: danakj <danakj@chromium.org>
Date: Thu Dec 13 23:20:01 2018

Remove conditional check to avoid crash when main frame is detached

This was handling the case where the main frame was detached without
freezing the RenderWidget. Now that that should no longer happen, as
the FrameMsg_Delete call was removed from ~RenderFrameHostImpl, we
should not need this branch anymore, and a RenderWidget should always
be frozen when there is no frame (and no WebFrameWidget, currently)
around.

If something is strange then we'll see crashes start happening in
WebViewImpl::GetURLForDebugTrace() again, but we expect to not.

TBR=dcheng@chromium.org

Change-Id: I4212622dbb8cbe5a6855d2d345490a5dbbf87683
Bug: 906340
Reviewed-on: https://chromium-review.googlesource.com/c/1377103
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616479}
[modify] https://crrev.com/f51754379ee4b2d22db968d0c8ea21754b300246/content/renderer/render_widget.cc

Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/b18768f234966f5aca498cae15c7f405d273486a

Commit: b18768f234966f5aca498cae15c7f405d273486a
Author: danakj@chromium.org
Commiter: danakj@chromium.org
Date: 2018-12-07 17:01:11 +0000 UTC

For M72 branch, avoid crashing when there's no main frame around.

Sometimes we get a request for a compositor frame sink when the main
frame is not present and this would crash. Remove the debugging CHECK()
and handle the case for merging to the M72 branch. Will revert again
back on trunk, as dcheng@ works toward removing the one case we believe
can cause this.

R=piman@chromium.org
TBR=danakj@chromium.org

(cherry picked from commit be7cebf02ef4a1cfc740a3ff30db71597772aba2)

Change-Id: I89686a3c2f83156f72e77940099be56bcc401d57
Bug: 906340
Reviewed-on: https://chromium-review.googlesource.com/c/1362004
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613780}
Reviewed-on: https://chromium-review.googlesource.com/c/1368025
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#135}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Sign in to add a comment