Detach old frame on WebFrame::swap.

Source/web/tests/WebFrameTest.cpp

 /*
  * Copyright (C) 2010 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 7004 matching lines @@
     ASSERT_TRUE(windowTop->IsObject());
     v8::Local<v8::Value> originalWindow = mainFrame()->executeScriptAndReturnValue(WebScriptSource(
         "document.querySelector('#frame2').contentWindow;"));
     ASSERT_TRUE(originalWindow->IsObject());
 
     // Make sure window reference stays the same when swapping to a remote frame.
     FrameTestHelpers::TestWebRemoteFrameClient remoteClient;
     WebRemoteFrame* remoteFrame = remoteClient.frame();
     WebFrame* targetFrame = mainFrame()->firstChild()->nextSibling();
     targetFrame->swap(remoteFrame);
-    // FIXME: This cleanup should be unnecessary, but the interaction between frame detach
-    // and swap is completely broken atm. swap() calls detachChildren() on the frame being
-    // swapped out, and frame B has a child. When the child of B is detached, it schedules a
-    // FrameLoader timer to check for load completion in its parent.
-    // Other tests end up spinning the message loop (e.g. to wait for load completion of
-    // another test page, so this timer callback is processed before FrameHost and Page are
-    // destroyed by the reset() at the end of the test case. However, this test does not spin
-    // the event loop in its body, so the timer task remains in the queue and ends up running
-    // in the setup for the following test. Since the FrameHost/Page for the associated
-    // FrameLoader have already been destroyed, this leads to crashes/use-after-frees. To
-    // prevent that, manually call detach() on the Frame to release its resources and cancel
-    // pending callbacks.
-    toCoreFrame(targetFrame)->detach();
     remoteFrame->setReplicatedOrigin(SecurityOrigin::createUnique());
     v8::Local<v8::Value> remoteWindow = mainFrame()->executeScriptAndReturnValue(WebScriptSource(
         "document.querySelector('#frame2').contentWindow;"));
     EXPECT_TRUE(originalWindow->StrictEquals(remoteWindow));
     // Check that its view is consistent with the world.
     v8::Local<v8::Value> remoteWindowTop = mainFrame()->executeScriptAndReturnValue(WebScriptSource(
         "document.querySelector('#frame2').contentWindow.top;"));
     EXPECT_TRUE(windowTop->StrictEquals(remoteWindowTop));
 
     // Now check that remote -> local works too, since it goes through a different code path.
     FrameTestHelpers::TestWebFrameClient client;
     WebLocalFrame* localFrame = WebLocalFrame::create(WebTreeScopeType::Document, &client);
     localFrame->initializeToReplaceRemoteFrame(remoteFrame, "", WebSandboxFlags::None);
     remoteFrame->swap(localFrame);
     v8::Local<v8::Value> localWindow = mainFrame()->executeScriptAndReturnValue(WebScriptSource(
         "document.querySelector('#frame2').contentWindow;"));
     EXPECT_TRUE(originalWindow->StrictEquals(localWindow));
     v8::Local<v8::Value> localWindowTop = mainFrame()->executeScriptAndReturnValue(WebScriptSource(
         "document.querySelector('#frame2').contentWindow.top;"));
     EXPECT_TRUE(windowTop->StrictEquals(localWindowTop));
 
     // Manually reset to break WebViewHelper's dependency on the stack allocated
     // TestWebFrameClient.
     reset();
-    remoteFrame->close();
 }
 
 TEST_F(WebFrameSwapTest, SwapInitializesGlobal)
 {
     v8::HandleScope scope(v8::Isolate::GetCurrent());
 
     v8::Local<v8::Value> windowTop = mainFrame()->executeScriptAndReturnValue(WebScriptSource("window"));
     ASSERT_TRUE(windowTop->IsObject());
 
     v8::Local<v8::Value> lastChild = mainFrame()->executeScriptAndReturnValue(WebScriptSource("saved = window[2]"));
@@ skipping 639 matching lines @@
     ScrollUpdate(&webViewHelper, 30, 0);
     Mock::VerifyAndClearExpectations(&client);
 
     // Overscroll is not reported.
     EXPECT_CALL(client, didOverscroll(WebFloatSize(), WebFloatSize(), WebFloatPoint(33, 33), WebFloatSize())).Times(0);
     ScrollEnd(&webViewHelper);
     Mock::VerifyAndClearExpectations(&client);
 }
 
 } // namespace blink