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

Issue 651980 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

DCHECK(!frame_widget_) in RenderViewImpl::AttachWebFrameWidget

Project Member Reported by alex...@chromium.org, Sep 30 2016

Issue description

On Linux debug build (mine was synced to r421582):
1. Create a fresh profile, and log in to both google.com and chromium.org accounts.
2. Install Google Drive app. (https://chrome.google.com/webstore/detail/apdfllckaahabafndbhieahigkjlhalf)
3. Restart the browser, and go to https://csreis.github.io.
4. Open DevTools and execute:
   window.open("https://drive.google.com/open?id=0B5cKk7MxQ2LsOG4wWmFxcHZfYXM")
5. This should bring you to account selection page on accounts.google.com.  Click on the google account.
6. Without waiting for video to load, quickly go to first tab, and re-execute the window.open.
7. Repeat 5-6 if there was no crash.  (It's racy and only happens sometimes, but it seems to require the accounts page to commit while the video is still loading.  I usually get the crash by the third try.)

Stack trace:
[1:1:0930/152909:FATAL:render_view_impl.cc(1658)] Check failed: !frame_widget_. 
#0 0x7f34d70dfece base::debug::StackTrace::StackTrace()
#1 0x7f34d714daaf logging::LogMessage::~LogMessage()
#2 0x7f34d2a34b53 content::RenderViewImpl::AttachWebFrameWidget()
#3 0x7f34d2a53776 content::RenderWidget::CreateForFrame()
#4 0x7f34d299ea3f content::RenderFrameImpl::CreateFrame()
#5 0x7f34d2a08d92 content::RenderThreadImpl::OnCreateNewFrame()

--site-per-process is not required.

I discovered this as part of issue 627400; see comment 21.  

This might be related to  issue 548275 , which nasko@ fixed a while back, and which was hitting the same DCHECK.

lfg@/dcheng@: any ideas about this?  

I'm assigning this to myself for now, but I will first concentrate on issue 627400, so if someone who's worked on widgets wants to take a look at this in the meantime, please feel free to reassign.
 
Found a deterministic repro for this.  Follow original repro steps through step 5.  Then:
6.  Go to first tab and re-execute window.open.  This should open another tab and again bring you to the account selection page.
7.  On this new tab, open DevTools and execute 
location.href="https://drive.google.com/open?id=0B5cKk7MxQ2LsOG4wWmFxcHZfYXM"

This causes the third tab (which is now showing the video from step 5) to die.
Cc: jam@chromium.org
Another, much simpler way to repro: run ProcessManagerBrowserTest.NestedURLNavigationsToExtensionBlocked with
--enable-browser-side-navigation.

Status: Started (was: Assigned)
I've got a fix in progress for this.  This happens because when we cancel the pending RFH in DiscardUnusedFrame for a main frame, we destroy the RFHI but never delete the main frame's RenderFrame and its widget on the renderer side (since the RenderView sticks around and is reused by the proxy we leave behind).  Later, in the repro steps, we try to recreate the main frame and crash because the widget already exists.
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 14 2016

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

commit 78c9c0d7d8e45837bc93efababf7fe517b2b36ad
Author: alexmos <alexmos@chromium.org>
Date: Fri Oct 14 18:57:03 2016

Fix RenderView reuse issues when canceling a pending RenderFrameHost.

This CL fixes three issues that happen when a pending RenderFrameHost
is canceled and discarded, but its SiteInstance has other active
frames.  In that case, the pending RenderFrameHost is replaced with a
proxy, so that other frames in the SiteInstance can still communicate
with it.

1) When a main frame pending RFH is canceled, we never update its
RVH's main_frame_routing_id() or is_active() status.  If the RVH is
later reused by another main frame navigation, these parameters would
be stale, leading to a crash in CreateRenderView trying to resolve
the main frame via the stale routing ID.  (Issue 627400)

2) The discarded RFH was deleted on the browser side, but the
corresponding RenderFrame wasn't deleted on the renderer side.  That
led to a renderer-side crash of trying to reuse the leaked
RenderFrame's widget if we ever re-navigated to the same SiteInstance
in the same frame.  The fix is to send a FrameMsg_Delete in the cases
where the discarded main frame isn't going to be cleaned up by its RVH
going away (in the case where the proxy is going to keep it
alive).  ( Issue 651980 )

3) The replacement proxy's RenderFrameProxyHost was created, but the
RenderFrameProxy wasn't actually initialized on the renderer side, so
other frames couldn't actually communicate with this frame.  (Issue
653746)

These changes are also likely to help with crashes we've been chasing
in issues 626802 and  575245 .

BUG=627400, 651980 , 653746 ,626802, 575245 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2410153005
Cr-Commit-Position: refs/heads/master@{#425409}

[modify] https://crrev.com/78c9c0d7d8e45837bc93efababf7fe517b2b36ad/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/78c9c0d7d8e45837bc93efababf7fe517b2b36ad/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://crrev.com/78c9c0d7d8e45837bc93efababf7fe517b2b36ad/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/78c9c0d7d8e45837bc93efababf7fe517b2b36ad/content/renderer/render_frame_proxy.cc
[modify] https://crrev.com/78c9c0d7d8e45837bc93efababf7fe517b2b36ad/content/renderer/render_widget.cc
[modify] https://crrev.com/78c9c0d7d8e45837bc93efababf7fe517b2b36ad/content/renderer/render_widget.h

Status: Fixed (was: Started)
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 4 2017

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

commit ab33b38dd1014114876adbcd70bcea76d5b1e614
Author: jiaxi <jiaxi@google.com>
Date: Wed Jan 04 05:51:17 2017

[MD Bookmarks] Add UI for Material Bookmarks.

This CL adds the basic UI for the material design bookmark manager.
- list.html is the list of bookmarks.
- sidebar.html contains the folder tree.

BUG= 651980 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2603303002
Cr-Commit-Position: refs/heads/master@{#441325}

[modify] https://crrev.com/ab33b38dd1014114876adbcd70bcea76d5b1e614/chrome/browser/browser_resources.grd
[modify] https://crrev.com/ab33b38dd1014114876adbcd70bcea76d5b1e614/chrome/browser/resources/md_bookmarks/app.html
[add] https://crrev.com/ab33b38dd1014114876adbcd70bcea76d5b1e614/chrome/browser/resources/md_bookmarks/folder_node.html
[add] https://crrev.com/ab33b38dd1014114876adbcd70bcea76d5b1e614/chrome/browser/resources/md_bookmarks/folder_node.js
[add] https://crrev.com/ab33b38dd1014114876adbcd70bcea76d5b1e614/chrome/browser/resources/md_bookmarks/icons.html
[add] https://crrev.com/ab33b38dd1014114876adbcd70bcea76d5b1e614/chrome/browser/resources/md_bookmarks/item.html
[add] https://crrev.com/ab33b38dd1014114876adbcd70bcea76d5b1e614/chrome/browser/resources/md_bookmarks/item.js
[add] https://crrev.com/ab33b38dd1014114876adbcd70bcea76d5b1e614/chrome/browser/resources/md_bookmarks/list.html
[add] https://crrev.com/ab33b38dd1014114876adbcd70bcea76d5b1e614/chrome/browser/resources/md_bookmarks/list.js
[add] https://crrev.com/ab33b38dd1014114876adbcd70bcea76d5b1e614/chrome/browser/resources/md_bookmarks/sidebar.html
[add] https://crrev.com/ab33b38dd1014114876adbcd70bcea76d5b1e614/chrome/browser/resources/md_bookmarks/sidebar.js
[modify] https://crrev.com/ab33b38dd1014114876adbcd70bcea76d5b1e614/chrome/browser/ui/webui/md_bookmarks/md_bookmarks_ui.cc
[add] https://crrev.com/ab33b38dd1014114876adbcd70bcea76d5b1e614/chrome/test/data/webui/md_bookmarks/item_test.js
[modify] https://crrev.com/ab33b38dd1014114876adbcd70bcea76d5b1e614/chrome/test/data/webui/md_bookmarks/md_bookmarks_browsertest.js
[add] https://crrev.com/ab33b38dd1014114876adbcd70bcea76d5b1e614/chrome/test/data/webui/md_bookmarks/sidebar_test.js

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 6 2017

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

commit 98f40af6044a40ba8a10899efb11d75ab122dffd
Author: jiaxi <jiaxi@google.com>
Date: Fri Jan 06 03:36:38 2017

[MD Bookmarks] Add Delete and Copy URL for Material Bookmarks.

This CL adds delete bookmarks and folders function and copy URL function.

BUG= 651980 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2613683002
Cr-Commit-Position: refs/heads/master@{#441864}

[modify] https://crrev.com/98f40af6044a40ba8a10899efb11d75ab122dffd/chrome/browser/resources/md_bookmarks/item.js
[modify] https://crrev.com/98f40af6044a40ba8a10899efb11d75ab122dffd/chrome/browser/resources/md_bookmarks/list.js
[modify] https://crrev.com/98f40af6044a40ba8a10899efb11d75ab122dffd/chrome/browser/resources/md_bookmarks/store.js
[modify] https://crrev.com/98f40af6044a40ba8a10899efb11d75ab122dffd/chrome/test/data/webui/md_bookmarks/store_test.js

Project Member

Comment 9 by bugdroid1@chromium.org, Jan 9 2017

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

commit 2f0417037d5a8bef540db234e7080c176fde1f5a
Author: jiaxi <jiaxi@google.com>
Date: Mon Jan 09 03:50:05 2017

[MD Bookmarks] Fix Closure Problem.

This CL fixes the closure issue generated by issue 2613943003.

BUG= 651980 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2619183002
Cr-Commit-Position: refs/heads/master@{#442192}

[modify] https://crrev.com/2f0417037d5a8bef540db234e7080c176fde1f5a/chrome/browser/resources/md_bookmarks/app.js

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 10 2017

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

commit f0fbe3213ad51e6d6404c10f6de9cc9461cff1fa
Author: jiaxi <jiaxi@google.com>
Date: Tue Jan 10 04:37:41 2017

[MD Bookmarks] Edit Bookmarks.

This CL enables edit function for bookmarks. When the edit button in the
dropdown menu for bookmarks is tapped, it will open an edit dialog.

BUG= 651980 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2617943002
Cr-Commit-Position: refs/heads/master@{#442470}

[modify] https://crrev.com/f0fbe3213ad51e6d6404c10f6de9cc9461cff1fa/chrome/browser/resources/md_bookmarks/list.html
[modify] https://crrev.com/f0fbe3213ad51e6d6404c10f6de9cc9461cff1fa/chrome/browser/resources/md_bookmarks/list.js
[modify] https://crrev.com/f0fbe3213ad51e6d6404c10f6de9cc9461cff1fa/chrome/browser/resources/md_bookmarks/shared_style.html
[modify] https://crrev.com/f0fbe3213ad51e6d6404c10f6de9cc9461cff1fa/chrome/browser/resources/md_bookmarks/shared_vars.html
[modify] https://crrev.com/f0fbe3213ad51e6d6404c10f6de9cc9461cff1fa/chrome/browser/resources/md_bookmarks/store.js
[modify] https://crrev.com/f0fbe3213ad51e6d6404c10f6de9cc9461cff1fa/chrome/browser/ui/webui/md_bookmarks/md_bookmarks_ui.cc
[modify] https://crrev.com/f0fbe3213ad51e6d6404c10f6de9cc9461cff1fa/chrome/test/data/webui/md_bookmarks/store_test.js
[modify] https://crrev.com/f0fbe3213ad51e6d6404c10f6de9cc9461cff1fa/components/bookmark_component_strings.grdp

Project Member

Comment 11 by bugdroid1@chromium.org, Jan 10 2017

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

commit 236b242417b64ccb62084dd6a6b63f50074b9897
Author: jiaxi <jiaxi@google.com>
Date: Tue Jan 10 07:34:39 2017

[MD WebUI] Move icons used in Bookmarks and Settings to shared file.

This CL moves everything in icons.html from bookmarks and settings to
cr_elements/icons.html and updates the related files using it.

BUG= 651980 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2615673003
Cr-Commit-Position: refs/heads/master@{#442525}

[modify] https://crrev.com/236b242417b64ccb62084dd6a6b63f50074b9897/chrome/browser/resources/md_bookmarks/folder_node.js
[modify] https://crrev.com/236b242417b64ccb62084dd6a6b63f50074b9897/chrome/browser/resources/md_bookmarks/icons.html
[modify] https://crrev.com/236b242417b64ccb62084dd6a6b63f50074b9897/chrome/browser/resources/md_bookmarks/item.html
[modify] https://crrev.com/236b242417b64ccb62084dd6a6b63f50074b9897/chrome/browser/resources/settings/basic_page/basic_page.js
[modify] https://crrev.com/236b242417b64ccb62084dd6a6b63f50074b9897/chrome/browser/resources/settings/icons.html
[modify] https://crrev.com/236b242417b64ccb62084dd6a6b63f50074b9897/chrome/browser/resources/settings/people_page/change_picture.html
[modify] https://crrev.com/236b242417b64ccb62084dd6a6b63f50074b9897/chrome/browser/resources/settings/settings_menu/settings_menu.js
[modify] https://crrev.com/236b242417b64ccb62084dd6a6b63f50074b9897/ui/webui/resources/cr_elements/icons.html

Sign in to add a comment