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

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

Created:
5 years, 7 months ago by alexmos
Modified:
5 years, 7 months ago
Reviewers:
nasko, piman
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 Revert of OOPIF: Specify previous sibling frames when creating new RenderFrames. (patchset #1 id:1 of https://codereview.chromium.org/1118083004/) Reason for revert: 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/. Original issue's 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} TBR=nasko@chromium.org,piman@chromium.org BUG=478792 Committed: https://crrev.com/9f8705a9018d34dc5d7b834ebfc60d36b8208b5f Cr-Commit-Position: refs/heads/master@{#328591}

Patch Set 1 #

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

Messages

Total messages: 5 (1 generated)
alexmos
Created Revert of Revert of OOPIF: Specify previous sibling frames when creating new RenderFrames.
5 years, 7 months ago (2015-05-06 18:09:43 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1131593003/1
5 years, 7 months ago (2015-05-06 18:13:26 UTC) #3
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 7 months ago (2015-05-06 19:59:06 UTC) #4
commit-bot: I haz the power
5 years, 7 months ago (2015-05-06 20:00:10 UTC) #5
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/9f8705a9018d34dc5d7b834ebfc60d36b8208b5f
Cr-Commit-Position: refs/heads/master@{#328591}

Powered by Google App Engine
This is Rietveld 408576698