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

Issue 1092973002: Revert of PlzNavigate: move ownership of the NavigationRequest to the FrameTreeNode (Closed)

Created:
5 years, 8 months ago by johnme
Modified:
5 years, 8 months ago
CC:
chromium-reviews, jam, nasko+codewatch_chromium.org, creis+watch_chromium.org, darin-cc_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 PlzNavigate: move ownership of the NavigationRequest to the FrameTreeNode (patchset #5 id:80001 of https://codereview.chromium.org/1080073004/) Reason for revert: Tentative revert to try to fix fast/dom/Window/new-window-opener.html on Mac, which has been crashing consistently since http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.9/builds/18168 Original issue's description: > PlzNavigate: move ownership of the NavigationRequest to the FrameTreeNode > > This CL moves the ownership of NavigationRequests from a map in NavigatorImpl > to the FrameTreeNode. This ensures that NavigationRequests are unique per > FrameTreeNode and are also properly deleted on destruction of the > FrameTreeNode. It also makes it easier to have PlzNavigate adapt to the > refactoring of DidStart/StopLoading happening in > https://codereview.chromium.org/1080143003/. > > BUG=439423 > > Committed: https://crrev.com/dcb434c11019afedcd5e45820e724b8ad9e6d396 > Cr-Commit-Position: refs/heads/master@{#325498} TBR=nasko@chromium.org,fdegans@chromium.org,carlosk@chromium.org,clamy@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=439423

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+109 lines, -94 lines) Patch
M content/browser/frame_host/frame_tree_node.h View 3 chunks +0 lines, -20 lines 0 comments Download
M content/browser/frame_host/frame_tree_node.cc View 3 chunks +4 lines, -24 lines 0 comments Download
M content/browser/frame_host/navigation_controller_impl_unittest.cc View 2 chunks +7 lines, -4 lines 0 comments Download
M content/browser/frame_host/navigator_impl.h View 2 chunks +10 lines, -0 lines 0 comments Download
M content/browser/frame_host/navigator_impl.cc View 8 chunks +43 lines, -12 lines 0 comments Download
M content/browser/frame_host/navigator_impl_unittest.cc View 34 chunks +42 lines, -33 lines 0 comments Download
M content/test/test_render_frame_host.cc View 1 chunk +3 lines, -1 line 0 comments Download

Messages

Total messages: 7 (2 generated)
johnme
Created Revert of PlzNavigate: move ownership of the NavigationRequest to the FrameTreeNode
5 years, 8 months ago (2015-04-17 15:57:00 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1092973002/1
5 years, 8 months ago (2015-04-17 15:57:55 UTC) #2
commit-bot: I haz the power
Failed to apply patch for content/browser/frame_host/frame_tree_node.cc: While running git apply --index -3 -p1; error: patch ...
5 years, 8 months ago (2015-04-17 15:59:32 UTC) #4
clamy
Hey John, I think it is unlikely that my CL is the cause of failure ...
5 years, 8 months ago (2015-04-17 16:57:12 UTC) #6
schenney
5 years, 8 months ago (2015-04-17 19:49:39 UTC) #7
On 2015/04/17 16:57:12, clamy wrote:
> Hey John, I think it is unlikely that my CL is the cause of failure of the
bot.
> This CL only contains code that should be executed with a specific flag on. In
> particular this flag is not passed to any bot except the fyi bot Browser Side
> Navigation Linux. So it should absolutely note execute on any Blink bot (they
> are CHECKs in the code to enforce that). Also note that
> https://codereview.chromium.org/1080143003/ landed in the meantime and was
> rebased on top of this patch.

I agree. I think the crashes may be due to the same thing that is causing
browser_test failures.

Powered by Google App Engine
This is Rietveld 408576698