Skip to content

Commit

Permalink
Revert of Revert of Set frame liveness before calling RenderFrameCrea…
Browse files Browse the repository at this point in the history
…ted/RenderFrameDeleted. (patchset #1 id:1 of https://codereview.chromium.org/1086613003/)

Reason for revert:
Relanding because the test is still failing even with this patch reverted.

Original issue's description:
> Revert of Set frame liveness before calling RenderFrameCreated/RenderFrameDeleted. (patchset #2 id:20001 of https://codereview.chromium.org/1070553005/)
>
> Reason for revert:
> Tentative revert to try to fix BrowserTest.WindowOpenClose, which is consistently failing on chromium.webkit Mac10.6 Tests and Mac10.8 Tests.
>
> It times out during ui_test_utils::NavigateToURLWithDispositionBlockUntilNavigationsComplete and gets killed whilst waiting for the navigations.
>
> It started failing between https://build.chromium.org/p/chromium.webkit/builders/Mac10.8%20Tests/builds/474 and https://build.chromium.org/p/chromium.webkit/builders/Mac10.6%20Tests/builds/29002, so the blamelists are:
>
> Chromium: https://chromium.googlesource.com/chromium/src.git/+log/96626920573e30a2cd02040b7ccec2c56f38b4db..274eb3202c59d4842c191e924577e81f14044af5
>
> Blink: https://chromium.googlesource.com/chromium/blink/+log/7f9db67d6a96399f8e4eae490efa83aa1b59107f..4dd6623614cff05eb3c18cd96f8d6ed089b7965b
>
> This is the only patch that seems at all relevant from amongst those...
>
> Original issue's description:
> > Set frame liveness before calling RenderFrameCreated/RenderFrameDeleted.
> >
> > In the FrameTree unittests, illustrate the liveness of frames in the ASCII
> > dumps. Update some tests so that children aren't born to dead parents.
> >
> > BUG=474231
> >
> > Committed: https://crrev.com/ad74a7e371149624d91f96cf662f70af6d18787f
> > Cr-Commit-Position: refs/heads/master@{#325508}
>
> TBR=nasko@chromium.org,nick@chromium.org
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=474231
>
> Committed: https://crrev.com/5e69a55db837879b5d4ca4b0935cf61742e09751
> Cr-Commit-Position: refs/heads/master@{#325629}

TBR=nasko@chromium.org,nick@chromium.org,johnme@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=474231

Review URL: https://codereview.chromium.org/1091303003

Cr-Commit-Position: refs/heads/master@{#325665}
  • Loading branch information
schenney-chromium authored and Commit bot committed Apr 17, 2015
1 parent 1c0cfff commit 6408fed
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 4 deletions.
11 changes: 9 additions & 2 deletions content/browser/frame_host/frame_tree_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ namespace {
void AppendTreeNodeState(FrameTreeNode* node, std::string* result) {
result->append(
base::Int64ToString(node->current_frame_host()->GetRoutingID()));
if (!node->current_frame_host()->IsRenderFrameLive())
result->append("*"); // Asterisk next to dead frames.

if (!node->frame_name().empty()) {
result->append(" '");
result->append(node->frame_name());
Expand Down Expand Up @@ -202,11 +205,12 @@ TEST_F(FrameTreeTest, DISABLED_Shape) {
// WebContentsObservers see a consistent view of the tree as we go.
TEST_F(FrameTreeTest, ObserverWalksTreeDuringFrameCreation) {
TreeWalkingWebContentsLogger activity(contents());
contents()->NavigateAndCommit(GURL("http://www.google.com"));
EXPECT_EQ("", activity.GetLog());

FrameTree* frame_tree = contents()->GetFrameTree();
FrameTreeNode* root = frame_tree->root();

EXPECT_EQ("", activity.GetLog());

// Simulate attaching a series of frames to build the frame tree.
main_test_rfh()->OnCreateChildFrame(14, std::string(), SandboxFlags::NONE);
EXPECT_EQ(
Expand All @@ -228,6 +232,8 @@ TEST_F(FrameTreeTest, ObserverWalksTreeDuringFrameCreation) {
// recovery from a render process crash.
TEST_F(FrameTreeTest, ObserverWalksTreeAfterCrash) {
TreeWalkingWebContentsLogger activity(contents());
contents()->NavigateAndCommit(GURL("http://www.google.com"));
EXPECT_EQ("", activity.GetLog());

main_test_rfh()->OnCreateChildFrame(22, std::string(), SandboxFlags::NONE);
EXPECT_EQ(
Expand Down Expand Up @@ -255,6 +261,7 @@ TEST_F(FrameTreeTest, ObserverWalksTreeAfterCrash) {
// Ensure that frames are not added to the tree, if the process passed in
// is different than the process of the parent node.
TEST_F(FrameTreeTest, FailAddFrameWithWrongProcessId) {
contents()->NavigateAndCommit(GURL("http://www.google.com"));
FrameTree* frame_tree = contents()->GetFrameTree();
FrameTreeNode* root = frame_tree->root();
int process_id = root->current_frame_host()->GetProcess()->GetID();
Expand Down
6 changes: 4 additions & 2 deletions content/browser/frame_host/render_frame_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -628,16 +628,18 @@ bool RenderFrameHostImpl::IsRenderFrameLive() {
}

void RenderFrameHostImpl::SetRenderFrameCreated(bool created) {
bool was_created = render_frame_created_;
render_frame_created_ = created;

// If the current status is different than the new status, the delegate
// needs to be notified.
if (delegate_ && (created != render_frame_created_)) {
if (delegate_ && (created != was_created)) {
if (created)
delegate_->RenderFrameCreated(this);
else
delegate_->RenderFrameDeleted(this);
}

render_frame_created_ = created;
if (created && render_widget_host_)
render_widget_host_->InitForFrame();
}
Expand Down

0 comments on commit 6408fed

Please sign in to comment.