Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(116)

Issue 1118083004: Revert of OOPIF: Specify previous sibling frames when creating new RenderFrames. (Closed)

Created:
5 years, 7 months ago by piman
Modified:
5 years, 7 months ago
Reviewers:
alexmos, nasko
CC:
chromium-reviews, mlamouri+watch-content_chromium.org, creis+watch_chromium.org, nasko+codewatch_chromium.org, jam, darin-cc_chromium.org, mkwst+moarreviews-renderer_chromium.org, site-isolation-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of OOPIF: Specify previous sibling frames when creating new RenderFrames. (patchset #5 id:80001 of https://codereview.chromium.org/1113393004/) Reason for revert: Causing failures on asan bots: https://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%20LSan%20Tests%20%281%29/builds/13202/steps/content_browsertests/logs/stdio https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/builds/1047/steps/content_browsertests/logs/stdio Original issue's description: > OOPIF: Specify previous sibling frames when creating RenderFrames. > > When initializing a new renderer for an OOP frame, the current > behavior is to first create all the RenderFrameProxies, and then to > create the new RenderFrame, appending it as its parent's last child in > the frame tree. This disregards the order of sibling frames and thus > may break indexed window access (e.g., window.frames[2]). > > This CL passes the previous sibling's routing ID in the > FrameMsg_NewFrame message, so that the new frame can be inserted in > the correct place in the frame tree. Note that we don't need to do > this for RenderFrameProxies, as those are already created in the > correct order (by CreateProxiesForSiteInstance) when initializing a > new renderer process. > > Corresponding Blink CL: https://codereview.chromium.org/1119823003/ > > BUG=478792 > > Committed: https://crrev.com/134cdb8c234847ebde156e46cad95be3221dc66b > Cr-Commit-Position: refs/heads/master@{#328384} TBR=nasko@chromium.org,alexmos@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=478792 Committed: https://crrev.com/e94c7ac058b50b1771a644b2d85652b725367bbd Cr-Commit-Position: refs/heads/master@{#328419}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+104 lines, -277 lines) Patch
M content/browser/frame_host/frame_tree_node.h View 1 chunk +0 lines, -4 lines 0 comments Download
M content/browser/frame_host/frame_tree_node.cc View 1 chunk +0 lines, -13 lines 0 comments Download
M content/browser/frame_host/frame_tree_unittest.cc View 1 chunk +0 lines, -24 lines 0 comments Download
M content/browser/frame_host/render_frame_host_impl.h View 1 chunk +1 line, -3 lines 0 comments Download
M content/browser/frame_host/render_frame_host_impl.cc View 2 chunks +12 lines, -17 lines 0 comments Download
M content/browser/frame_host/render_frame_host_manager.h View 1 chunk +0 lines, -1 line 0 comments Download
M content/browser/frame_host/render_frame_host_manager.cc View 2 chunks +3 lines, -19 lines 0 comments Download
M content/browser/site_per_process_browsertest.cc View 5 chunks +52 lines, -135 lines 0 comments Download
M content/browser/web_contents/web_contents_impl.h View 1 chunk +0 lines, -1 line 0 comments Download
M content/browser/web_contents/web_contents_impl.cc View 1 chunk +1 line, -3 lines 0 comments Download
M content/common/frame_messages.h View 2 chunks +13 lines, -31 lines 0 comments Download
M content/renderer/render_frame_impl.h View 1 chunk +6 lines, -8 lines 0 comments Download
M content/renderer/render_frame_impl.cc View 2 chunks +1 line, -9 lines 0 comments Download
M content/renderer/render_frame_impl_browsertest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M content/renderer/render_thread_impl.h View 2 chunks +6 lines, -2 lines 0 comments Download
M content/renderer/render_thread_impl.cc View 1 chunk +8 lines, -5 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
piman
Created Revert of OOPIF: Specify previous sibling frames when creating new RenderFrames.
5 years, 7 months ago (2015-05-05 22:28:24 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1118083004/1
5 years, 7 months ago (2015-05-05 22:28:59 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 7 months ago (2015-05-05 22:31:09 UTC) #3
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/e94c7ac058b50b1771a644b2d85652b725367bbd Cr-Commit-Position: refs/heads/master@{#328419}
5 years, 7 months ago (2015-05-05 22:32:03 UTC) #4
alexmos
5 years, 7 months ago (2015-05-06 18:09:42 UTC) #5
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/1131593003/ by alexmos@chromium.org.

The reason for reverting is: The reason for the original revert was a leak
caused by https://crbug.com/484760, and the relevant suppression was fixed in
https://codereview.chromium.org/1130603002/..

Powered by Google App Engine
This is Rietveld 408576698