Commit 292b63e6 authored by danakj's avatar danakj Committed by Commit Bot

Avoid racing RequestNewLayerTreeFrameSink with DoDeferredClose.

CloseWidgetSoon will post DoDeferredClose, which will stop new frame
sinks from being made.

However WebPagePopupImpl destroys its Page/MainFrame etc when it calls
CloseWidgetSoon, which also detaches the WebFrameWidget. Then if a
RequestNewLayerTreeFrameSink was already in the task queue, it would
run before DoDeferredClose, and crash when it tries to get the URL from
the non-existant WebFrameWidget/MainFrame.

R=piman@chromium.org

Change-Id: I94ec48915b6dba207f783a9d3b380476e97da9da
Bug: 896836
Reviewed-on: https://chromium-review.googlesource.com/c/1342774Reviewed-by: default avatarAntoine Labour <piman@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609423}
parent f0ccc185
...@@ -662,7 +662,8 @@ void RenderWidget::OnClose() { ...@@ -662,7 +662,8 @@ void RenderWidget::OnClose() {
DCHECK(content::RenderThread::Get()); DCHECK(content::RenderThread::Get());
if (closing_) if (closing_)
return; return;
NotifyOnClose(); for (auto& observer : render_frames_)
observer.WidgetWillClose();
closing_ = true; closing_ = true;
// Browser correspondence is no longer needed at this point. // Browser correspondence is no longer needed at this point.
...@@ -1604,17 +1605,9 @@ void RenderWidget::SetIsFrozen(bool is_frozen) { ...@@ -1604,17 +1605,9 @@ void RenderWidget::SetIsFrozen(bool is_frozen) {
} }
void RenderWidget::DoDeferredClose() { void RenderWidget::DoDeferredClose() {
// Prevent compositor from setting up new IPC channels, since we know a
// WidgetMsg_Close is coming.
host_will_close_this_ = true;
Send(new WidgetHostMsg_Close(routing_id_)); Send(new WidgetHostMsg_Close(routing_id_));
} }
void RenderWidget::NotifyOnClose() {
for (auto& observer : render_frames_)
observer.WidgetWillClose();
}
void RenderWidget::CloseWidgetSoon() { void RenderWidget::CloseWidgetSoon() {
DCHECK(content::RenderThread::Get()); DCHECK(content::RenderThread::Get());
if (is_frozen_) { if (is_frozen_) {
...@@ -1625,9 +1618,17 @@ void RenderWidget::CloseWidgetSoon() { ...@@ -1625,9 +1618,17 @@ void RenderWidget::CloseWidgetSoon() {
return; return;
} }
// Prevent compositor from setting up new IPC channels, since we know a
// WidgetMsg_Close is coming. We do this immediately, not in DoDeferredClose,
// as the caller (eg WebPagePopupImpl) may start tearing down things after
// calling this method, including detaching the frame from this RenderWidget.
// Then trying to make a LayerTreeFrameSink would crash.
// https://crbug.com/906340
host_will_close_this_ = true;
// If a page calls window.close() twice, we'll end up here twice, but that's // If a page calls window.close() twice, we'll end up here twice, but that's
// OK. It is safe to send multiple Close messages. // OK. It is safe to send multiple Close messages.
//
// Ask the RenderWidgetHost to initiate close. We could be called from deep // Ask the RenderWidgetHost to initiate close. We could be called from deep
// in Javascript. If we ask the RendwerWidgetHost to close now, the window // in Javascript. If we ask the RendwerWidgetHost to close now, the window
// could be closed before the JS finishes executing. So instead, post a // could be closed before the JS finishes executing. So instead, post a
......
...@@ -584,7 +584,6 @@ class CONTENT_EXPORT RenderWidget ...@@ -584,7 +584,6 @@ class CONTENT_EXPORT RenderWidget
void StopCompositor(); void StopCompositor();
void DoDeferredClose(); void DoDeferredClose();
void NotifyOnClose();
gfx::Size GetSizeForWebWidget() const; gfx::Size GetSizeForWebWidget() const;
void ResizeWebWidget(); void ResizeWebWidget();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment