CHECK failure: !render_view->main_render_frame_ in render_frame_impl.cc |
||||||||||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=4576166574555136 Fuzzer: mbarbella_js_mutation_layout Job Type: linux_msan_content_shell_drt Platform Id: linux Crash Type: CHECK failure Crash Address: Crash State: !render_view->main_render_frame_ in render_frame_impl.cc content::RenderFrameImpl::OnSwapOut bool IPC::MessageT<FrameMsg_SwapOut_Meta, std::__1::tuple<int, bool, content::Fr Sanitizer: memory (MSAN) Regressed: https://clusterfuzz.com/revisions?job=linux_msan_content_shell_drt&range=606033:606039 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=4576166574555136 Issue filed automatically. See https://github.com/google/clusterfuzz-tools for more information.
,
Nov 20
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/f8840b9771e2686d3054a311440abb48ac636e65 (Keep subframe alive in pending deletion.). 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.
,
Nov 22
I don't have time to fix it immediately, but I will as soon as my other bugs are fixed. +CC: FYI.
,
Nov 22
I took a quick look:
Test case:
~~~
<a href="about:blank" id="a"></a>
<iframe id="iframe"></iframe>
<script>
iframe.contentWindow.onunload = () => {
a.click();
};
location.href = 'check-ping.php';
</script>
~~~
In RenderFrameImpl::Swapout(). It reaches:
~~~
// For main frames, the swap should have cleared the RenderView's pointer to
// this frame.
if (is_main_frame)
CHECK(!render_view->main_render_frame_);
~~~
It looks like an easy to fix bug. As soon as I understand what this check is about...
,
Nov 26
The NextAction date has arrived: 2018-11-26
,
Nov 26
I understand. 1) The document in the main frame runs its unload handler and detaches itself. It set main_render_frame_ to null. 2) Then the document in the subframe runs its unload handler and makes the main frame to navigate to about:blank. It sets main_render_frame_ to non-nullptr. 3) The CHECK is reached after leaving the Swap(..) function. Navigations in an unloading frame shouldn't be allowed. Fix + regression tests: https://chromium-review.googlesource.com/c/chromium/src/+/1350970
,
Nov 29
,
Nov 29
This issue is marked as a release blocker with no milestone associated. Please add an appropriate milestone. All release blocking issues should have milestones associated to it, so that the issue can tracked and the fixes can be pushed promptly. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 3
,
Dec 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/05083730be6c1218024b485584a440fb78e675f3 commit 05083730be6c1218024b485584a440fb78e675f3 Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Tue Dec 04 10:42:09 2018 Block new navigations while unloading. One hole in blocking navigation while unloading remained. It's when the navigation is triggered from the subframe's unload handler. It fixes bug 907106 and 773683. Test ContentBrowserTest.HistoryBackInUnloadCancelsReload is removed, because it was explicitly relying on bug 773683. Bug: 907106 ,773683 Change-Id: I65755f636bb258cc55fba9d6775eb101da31b735 Reviewed-on: https://chromium-review.googlesource.com/c/1350970 Reviewed-by: Camille Lamy <clamy@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#613498} [modify] https://crrev.com/05083730be6c1218024b485584a440fb78e675f3/content/browser/frame_host/navigation_controller_impl_browsertest.cc [modify] https://crrev.com/05083730be6c1218024b485584a440fb78e675f3/third_party/blink/renderer/core/loader/frame_loader.cc [add] https://crrev.com/05083730be6c1218024b485584a440fb78e675f3/third_party/blink/web_tests/http/tests/navigation/navigate-to-about-blank-from-subframe-unload-handler-cross-origin-expected.txt [add] https://crrev.com/05083730be6c1218024b485584a440fb78e675f3/third_party/blink/web_tests/http/tests/navigation/navigate-to-about-blank-from-subframe-unload-handler-cross-origin.html [add] https://crrev.com/05083730be6c1218024b485584a440fb78e675f3/third_party/blink/web_tests/http/tests/navigation/navigate-to-about-blank-from-subframe-unload-handler-same-origin-expected.txt [add] https://crrev.com/05083730be6c1218024b485584a440fb78e675f3/third_party/blink/web_tests/http/tests/navigation/navigate-to-about-blank-from-subframe-unload-handler-same-origin.html
,
Dec 4
I am requesting merging this change into M72: - This is a one line change, it is not very complicated. - It fixed two M72 stable blockers: bug 907106 and probably bug 904248. - It fixes an old bug (bug 773683). Malicous website are no more able to prevent the user from navigating away from they page. I will wait for this patch to be tested on canary for 4 days before going to M72.
,
Dec 4
ClusterFuzz has detected this issue as fixed in range 613497:613498. Detailed report: https://clusterfuzz.com/testcase?key=4576166574555136 Fuzzer: mbarbella_js_mutation_layout Job Type: linux_msan_content_shell_drt Platform Id: linux Crash Type: CHECK failure Crash Address: Crash State: !render_view->main_render_frame_ in render_frame_impl.cc content::RenderFrameImpl::OnSwapOut bool IPC::MessageT<FrameMsg_SwapOut_Meta, std::__1::tuple<int, bool, content::Fr Sanitizer: memory (MSAN) Regressed: https://clusterfuzz.com/revisions?job=linux_msan_content_shell_drt&range=606033:606039 Fixed: https://clusterfuzz.com/revisions?job=linux_msan_content_shell_drt&range=613497:613498 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=4576166574555136 See https://github.com/google/clusterfuzz-tools for more information. If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
,
Dec 4
Thanks ClusterFuzz! :-) Waiting on merging this patch into M72 before closing this bug. See comment 11.
,
Dec 4
ClusterFuzz testcase 4576166574555136 is verified as fixed, so closing issue as verified. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
,
Dec 4
,
Dec 4
Pls merge your change to M72 branch 3626 ASAP so we can pick it up for tomororw's dev release. Thank you.
,
Dec 5
I prepared and approved the cherry-pick here: https://chromium-review.googlesource.com/c/chromium/src/+/1361711 As I said in comment 11, I would like to be sure it behaves nicely on Canary first. It has never been used in any version so far. Can I wait for 2 days? Maybe I'm too careful, it's only a Dev release. Feel free to submit it immediately if you think it should land now.
,
Dec 5
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
,
Dec 5
Re #18, Sure, it can wait for 2 days, if you like to wait more even Monday morning is also fine.
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f3faa6ac1915d41cd4bb06adcc4d8b2802666f54 commit f3faa6ac1915d41cd4bb06adcc4d8b2802666f54 Author: Arthur Sonzogni <arthursonzogni@chromium.org> Date: Fri Dec 07 08:45:11 2018 Block new navigations while unloading. One hole in blocking navigation while unloading remained. It's when the navigation is triggered from the subframe's unload handler. It fixes bug 907106 and 773683. Test ContentBrowserTest.HistoryBackInUnloadCancelsReload is removed, because it was explicitly relying on bug 773683. Bug: 907106 ,773683 Change-Id: I65755f636bb258cc55fba9d6775eb101da31b735 Reviewed-on: https://chromium-review.googlesource.com/c/1350970 Reviewed-by: Camille Lamy <clamy@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#613498}(cherry picked from commit 05083730be6c1218024b485584a440fb78e675f3) Reviewed-on: https://chromium-review.googlesource.com/c/1361711 Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#132} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/f3faa6ac1915d41cd4bb06adcc4d8b2802666f54/content/browser/frame_host/navigation_controller_impl_browsertest.cc [modify] https://crrev.com/f3faa6ac1915d41cd4bb06adcc4d8b2802666f54/third_party/blink/renderer/core/loader/frame_loader.cc [add] https://crrev.com/f3faa6ac1915d41cd4bb06adcc4d8b2802666f54/third_party/blink/web_tests/http/tests/navigation/navigate-to-about-blank-from-subframe-unload-handler-cross-origin-expected.txt [add] https://crrev.com/f3faa6ac1915d41cd4bb06adcc4d8b2802666f54/third_party/blink/web_tests/http/tests/navigation/navigate-to-about-blank-from-subframe-unload-handler-cross-origin.html [add] https://crrev.com/f3faa6ac1915d41cd4bb06adcc4d8b2802666f54/third_party/blink/web_tests/http/tests/navigation/navigate-to-about-blank-from-subframe-unload-handler-same-origin-expected.txt [add] https://crrev.com/f3faa6ac1915d41cd4bb06adcc4d8b2802666f54/third_party/blink/web_tests/http/tests/navigation/navigate-to-about-blank-from-subframe-unload-handler-same-origin.html
,
Dec 7
,
Dec 7
The NextAction date has arrived: 2018-12-07
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f3faa6ac1915d41cd4bb06adcc4d8b2802666f54 Commit: f3faa6ac1915d41cd4bb06adcc4d8b2802666f54 Author: arthursonzogni@chromium.org Commiter: arthursonzogni@chromium.org Date: 2018-12-07 08:45:11 +0000 UTC Block new navigations while unloading. One hole in blocking navigation while unloading remained. It's when the navigation is triggered from the subframe's unload handler. It fixes bug 907106 and 773683. Test ContentBrowserTest.HistoryBackInUnloadCancelsReload is removed, because it was explicitly relying on bug 773683. Bug: 907106 ,773683 Change-Id: I65755f636bb258cc55fba9d6775eb101da31b735 Reviewed-on: https://chromium-review.googlesource.com/c/1350970 Reviewed-by: Camille Lamy <clamy@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#613498}(cherry picked from commit 05083730be6c1218024b485584a440fb78e675f3) Reviewed-on: https://chromium-review.googlesource.com/c/1361711 Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#132} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} |
||||||||||||||
►
Sign in to add a comment |
||||||||||||||
Comment 1 by ClusterFuzz
, Nov 20Labels: Test-Predator-Auto-Components