Refactor frame navigation/detach state cleanup to be more sane.

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 6562 matching lines @@
 
 // Make sure that an embedder-triggered detach with a remote frame parent
 // doesn't leave behind dangling pointers.
 TEST_F(WebFrameTest, EmbedderTriggeredDetachWithRemoteMainFrame)
 {
     // FIXME: Refactor some of this logic into WebViewHelper to make it easier to
     // write tests with a top-level remote frame.
     FrameTestHelpers::TestWebViewClient viewClient;
     FrameTestHelpers::TestWebRemoteFrameClient remoteClient;
     WebView* view = WebView::create(&viewClient);
-    view->setMainFrame(WebRemoteFrame::create(&remoteClient));
+    view->setMainFrame(remoteClient.frame());
     FrameTestHelpers::TestWebFrameClient childFrameClient;
     WebLocalFrame* childFrame = view->mainFrame()->toWebRemoteFrame()->createLocalChild("", WebSandboxFlags::None, &childFrameClient);
 
     // Purposely keep the LocalFrame alive so it's the last thing to be destroyed.
     RefPtrWillBePersistent<Frame> childCoreFrame = toCoreFrame(childFrame);
     view->close();
     childCoreFrame.clear();
 }
 
 class WebFrameSwapTest : public WebFrameTest {
@@ skipping 193 matching lines @@
 TEST_F(WebFrameSwapTest, SwapPreservesGlobalContext)
 {
     v8::HandleScope scope(v8::Isolate::GetCurrent());
     v8::Local<v8::Value> windowTop = mainFrame()->executeScriptAndReturnValue(WebScriptSource("window"));
     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.
-    WebRemoteFrame* remoteFrame = WebRemoteFrame::create(nullptr);
+    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
@@ skipping 26 matching lines @@
     // Manually reset to break WebViewHelper's dependency on the stack allocated
     // TestWebFrameClient.
     reset();
     remoteFrame->close();
 }
 
 TEST_F(WebFrameSwapTest, RemoteFramesAreIndexable)
 {
     v8::HandleScope scope(v8::Isolate::GetCurrent());
 
-    WebRemoteFrame* remoteFrame = WebRemoteFrame::create(nullptr);
+    FrameTestHelpers::TestWebRemoteFrameClient remoteClient;
+    WebRemoteFrame* remoteFrame = remoteClient.frame();
     mainFrame()->lastChild()->swap(remoteFrame);
     remoteFrame->setReplicatedOrigin(SecurityOrigin::createUnique());
     v8::Local<v8::Value> remoteWindow = mainFrame()->executeScriptAndReturnValue(WebScriptSource("window[2]"));
     EXPECT_TRUE(remoteWindow->IsObject());
     v8::Local<v8::Value> windowLength = mainFrame()->executeScriptAndReturnValue(WebScriptSource("window.length"));
     ASSERT_TRUE(windowLength->IsNumber());
     v8::Local<v8::Integer> windowLengthInteger = windowLength->ToInteger();
     EXPECT_EQ(3, windowLengthInteger->Value());
+
+    reset();
 }
 
 TEST_F(WebFrameSwapTest, RemoteFrameLengthAccess)
 {
     v8::HandleScope scope(v8::Isolate::GetCurrent());
 
-    WebRemoteFrame* remoteFrame = WebRemoteFrame::create(nullptr);
+    FrameTestHelpers::TestWebRemoteFrameClient remoteClient;
+    WebRemoteFrame* remoteFrame = remoteClient.frame();
     mainFrame()->lastChild()->swap(remoteFrame);
     remoteFrame->setReplicatedOrigin(SecurityOrigin::createUnique());
     v8::Local<v8::Value> remoteWindowLength = mainFrame()->executeScriptAndReturnValue(WebScriptSource("window[2].length"));
     ASSERT_TRUE(remoteWindowLength->IsNumber());
     v8::Local<v8::Integer> remoteWindowLengthInteger = remoteWindowLength->ToInteger();
     EXPECT_EQ(0, remoteWindowLengthInteger->Value());
+
+    reset();
 }
 
 TEST_F(WebFrameSwapTest, RemoteWindowNamedAccess)
 {
     v8::HandleScope scope(v8::Isolate::GetCurrent());
 
     // FIXME: Once OOPIF unit test infrastructure is in place, test that named
     // window access on a remote window works. For now, just test that accessing
     // a named property doesn't crash.
-    WebRemoteFrame* remoteFrame = WebRemoteFrame::create(nullptr);
+    FrameTestHelpers::TestWebRemoteFrameClient remoteClient;
+    WebRemoteFrame* remoteFrame = remoteClient.frame();
     mainFrame()->lastChild()->swap(remoteFrame);
     remoteFrame->setReplicatedOrigin(SecurityOrigin::createUnique());
     v8::Local<v8::Value> remoteWindowProperty = mainFrame()->executeScriptAndReturnValue(WebScriptSource("window[2].foo"));
     EXPECT_TRUE(remoteWindowProperty.IsEmpty());
+
+    reset();
 }
 
 // TODO(alexmos, dcheng): This test and some other OOPIF tests use
 // very little of the test fixture support in WebFrameSwapTest.  We should
 // clean these tests up.
 TEST_F(WebFrameSwapTest, FramesOfRemoteParentAreIndexable)
 {
     v8::HandleScope scope(v8::Isolate::GetCurrent());
 
-    WebRemoteFrame* remoteParentFrame = WebRemoteFrame::create(nullptr);
+    FrameTestHelpers::TestWebRemoteFrameClient remoteClient;
+    WebRemoteFrame* remoteParentFrame = remoteClient.frame();
     mainFrame()->swap(remoteParentFrame);
     remoteParentFrame->setReplicatedOrigin(SecurityOrigin::createUnique());
 
     FrameTestHelpers::TestWebFrameClient childFrameClient;
     WebLocalFrame* childFrame = remoteParentFrame->createLocalChild("", WebSandboxFlags::None, &childFrameClient);
     FrameTestHelpers::loadFrame(childFrame, m_baseURL + "subframe-hello.html");
 
     v8::Local<v8::Value> window = childFrame->executeScriptAndReturnValue(WebScriptSource("window"));
     v8::Local<v8::Value> childOfRemoteParent = childFrame->executeScriptAndReturnValue(WebScriptSource("parent.frames[0]"));
     EXPECT_TRUE(childOfRemoteParent->IsObject());
     EXPECT_TRUE(window->StrictEquals(childOfRemoteParent));
 
     v8::Local<v8::Value> windowLength = childFrame->executeScriptAndReturnValue(WebScriptSource("parent.frames.length"));
     ASSERT_TRUE(windowLength->IsNumber());
     v8::Local<v8::Integer> windowLengthInteger = windowLength->ToInteger();
     EXPECT_EQ(1, windowLengthInteger->Value());
 
-    // Manually reset to break WebViewHelper's dependency on the stack allocated
-    // TestWebFrameClient.
+    // Manually reset to break WebViewHelper's dependency on the stack allocated clients.
     reset();
-    remoteParentFrame->close();
 }
 
 class RemoteToLocalSwapWebFrameClient : public FrameTestHelpers::TestWebFrameClient {
 public:
     explicit RemoteToLocalSwapWebFrameClient(WebRemoteFrame* remoteFrame)
         : m_historyCommitType(WebHistoryInertCommit)
         , m_remoteFrame(remoteFrame)
     {
     }
 
@@ skipping 39 matching lines @@
 
     const WebURLRequest& lastRequest() const { return m_lastRequest; }
 
 private:
     WebURLRequest m_lastRequest;
 };
 
 TEST_F(WebFrameSwapTest, NavigateRemoteFrameViaLocation)
 {
     RemoteNavigationClient client;
-    WebRemoteFrame* remoteFrame = WebRemoteFrame::create(&client);
+    WebRemoteFrame* remoteFrame = client.frame();
     WebFrame* targetFrame = mainFrame()->firstChild();
     ASSERT_TRUE(targetFrame);
     targetFrame->swap(remoteFrame);
     ASSERT_TRUE(mainFrame()->firstChild());
     ASSERT_EQ(mainFrame()->firstChild(), remoteFrame);
 
     remoteFrame->setReplicatedOrigin(WebSecurityOrigin::createFromString("http://127.0.0.1"));
     mainFrame()->executeScript(WebScriptSource(
         "document.getElementsByTagName('iframe')[0].contentWindow.location = 'data:text/html,hi'"));
     ASSERT_FALSE(client.lastRequest().isNull());
     EXPECT_EQ(WebURL(toKURL("data:text/html,hi")), client.lastRequest().url());
 
     // Manually reset to break WebViewHelper's dependency on the stack allocated
     // TestWebFrameClient.
     reset();
-    remoteFrame->close();
 }
 
 class MockDocumentThreadableLoaderClient : public DocumentThreadableLoaderClient {
 public:
     MockDocumentThreadableLoaderClient() : m_failed(false) { }
     virtual void didFail(const ResourceError&) override { m_failed = true;}
 
     void reset() { m_failed = false; }
     bool failed() { return m_failed; }
 
@@ skipping 97 matching lines @@
     EXPECT_EQ(4u, frameClient.provisionalLoadCount());
     EXPECT_FALSE(frameClient.wasLastProvisionalLoadATransition());
     EXPECT_EQ(1u, frameClient.navigationalDataReceivedCount());
 }
 
 TEST_F(WebFrameTest, DetachRemoteFrame)
 {
     FrameTestHelpers::TestWebViewClient viewClient;
     FrameTestHelpers::TestWebRemoteFrameClient remoteClient;
     WebView* view = WebView::create(&viewClient);
-    view->setMainFrame(WebRemoteFrame::create(&remoteClient));
+    view->setMainFrame(remoteClient.frame());
     FrameTestHelpers::TestWebRemoteFrameClient childFrameClient;
     WebRemoteFrame* childFrame = view->mainFrame()->toWebRemoteFrame()->createRemoteChild("", WebSandboxFlags::None, &childFrameClient);
     childFrame->detach();
     view->close();
 }
 
 } // namespace blink