Always use the WebFrameWidget when attaching the root graphics

third_party/WebKit/Source/web/tests/WebFrameTest.cpp

Index: third_party/WebKit/Source/web/tests/WebFrameTest.cpp
diff --git a/third_party/WebKit/Source/web/tests/WebFrameTest.cpp b/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
index b2d4ad274c57325a801197399a9ac6e4980b9bcb..e7b68387781def92bd5cb5cb6b6d8acbda7bfcaa 100644
--- a/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
+++ b/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
@@ -1605,6 +1605,8 @@ TEST_F(WebFrameTest, FrameOwnerPropertiesMargin)
     properties.marginWidth = 11;
     properties.marginHeight = 22;
     WebLocalFrame* localFrame = FrameTestHelpers::createLocalChild(root, "frameName", nullptr, nullptr, properties);
+    FrameTestHelpers::TestWebWidgetClient widgetClient;
+    WebWidget* widget = WebFrameWidget::create(&widgetClient, localFrame);

[dcheng, 2016/06/08 00:10:17]

I wonder if we can make this simpler for test code by storing the WebFrameWidget
on TestWebFrameClient? FrameTestHelpers::createLocalChild() can be responsible
for setting this up. Obviously we'll need something a bit different for the main
frame eventually, but that seems like it should make it easier for test code to
avoid duplicating so much boilerplate. WDYT?

[lfg, 2016/06/08 21:09:47]

I think that's doable, but there's one test that needs to override the client
(GestureEventTestWebWidgetClient). What do you think of having an optional
argument in createLocalChild for the client, with a default of nullptr which
would give you the default TestWebWidgetClient? We would also need to add code
to TestWebWidgetClient to destroy itself, since we won't be closing
widget->close(), but I think that can be done similarly to how RenderWidget
destroys itself.

[dcheng, 2016/06/09 04:43:51]

That seems reasonable to me.

 
     registerMockedHttpURLLoad("frame_owner_properties.html");
     FrameTestHelpers::loadFrame(localFrame, m_baseURL + "frame_owner_properties.html");
@@ -1620,6 +1622,7 @@ TEST_F(WebFrameTest, FrameOwnerPropertiesMargin)
     EXPECT_NE(nullptr, frameView->horizontalScrollbar());
     EXPECT_NE(nullptr, frameView->verticalScrollbar());
 
+    widget->close();
     view->close();
 }
 
@@ -1637,6 +1640,8 @@ TEST_F(WebFrameTest, FrameOwnerPropertiesScrolling)
     // Turn off scrolling in the subframe.
     properties.scrollingMode = WebFrameOwnerProperties::ScrollingMode::AlwaysOff;
     WebLocalFrame* localFrame = FrameTestHelpers::createLocalChild(root, "frameName", nullptr, nullptr, properties);
+    FrameTestHelpers::TestWebWidgetClient widgetClient;
+    WebWidget* widget = WebFrameWidget::create(&widgetClient, localFrame);
 
     registerMockedHttpURLLoad("frame_owner_properties.html");
     FrameTestHelpers::loadFrame(localFrame, m_baseURL + "frame_owner_properties.html");
@@ -1649,6 +1654,7 @@ TEST_F(WebFrameTest, FrameOwnerPropertiesScrolling)
     EXPECT_EQ(nullptr, frameView->horizontalScrollbar());
     EXPECT_EQ(nullptr, frameView->verticalScrollbar());
 
+    widget->close();
     view->close();
 }
 
@@ -7135,6 +7141,8 @@ TEST_F(WebFrameSwapTest, SwapMainFrame)
 
     FrameTestHelpers::TestWebFrameClient client;
     WebLocalFrame* localFrame = WebLocalFrame::createProvisional(&client, remoteFrame, WebSandboxFlags::None);
+    FrameTestHelpers::TestWebWidgetClient widgetClient;
+    WebWidget* widget = WebFrameWidget::create(&widgetClient, localFrame);
     remoteFrame->swap(localFrame);
 
     // Finally, make sure an embedder triggered load in the local frame swapped
@@ -7144,6 +7152,8 @@ TEST_F(WebFrameSwapTest, SwapMainFrame)
     std::string content = WebFrameContentDumper::dumpWebViewAsText(webView(), 1024).utf8();
     EXPECT_EQ("hello", content);
 
+    widget->close();
+
     // Manually reset to break WebViewHelper's dependency on the stack allocated
     // TestWebFrameClient.
     reset();
@@ -7499,6 +7509,8 @@ TEST_F(WebFrameSwapTest, FramesOfRemoteParentAreIndexable)
     remoteParentFrame->setReplicatedOrigin(SecurityOrigin::createUnique());
 
     WebLocalFrame* childFrame = FrameTestHelpers::createLocalChild(remoteParentFrame);
+    FrameTestHelpers::TestWebWidgetClient widgetClient;
+    WebWidget* widget = WebFrameWidget::create(&widgetClient, childFrame);
     FrameTestHelpers::loadFrame(childFrame, m_baseURL + "subframe-hello.html");
 
     v8::Local<v8::Value> window = childFrame->executeScriptAndReturnValue(WebScriptSource("window"));
@@ -7510,6 +7522,8 @@ TEST_F(WebFrameSwapTest, FramesOfRemoteParentAreIndexable)
     ASSERT_TRUE(windowLength->IsInt32());
     EXPECT_EQ(1, windowLength.As<v8::Int32>()->Value());
 
+    widget->close();
+
     // Manually reset to break WebViewHelper's dependency on the stack allocated clients.
     reset();
 }
@@ -7525,6 +7539,8 @@ TEST_F(WebFrameSwapTest, FrameElementInFramesWithRemoteParent)
     remoteParentFrame->setReplicatedOrigin(SecurityOrigin::createUnique());
 
     WebLocalFrame* childFrame = FrameTestHelpers::createLocalChild(remoteParentFrame);
+    FrameTestHelpers::TestWebWidgetClient widgetClient;
+    WebWidget* widget = WebFrameWidget::create(&widgetClient, childFrame);
     FrameTestHelpers::loadFrame(childFrame, m_baseURL + "subframe-hello.html");
 
     v8::Local<v8::Value> frameElement = childFrame->executeScriptAndReturnValue(WebScriptSource("window.frameElement"));
@@ -7532,6 +7548,8 @@ TEST_F(WebFrameSwapTest, FrameElementInFramesWithRemoteParent)
     ASSERT_FALSE(frameElement.IsEmpty());
     EXPECT_TRUE(frameElement->IsNull());
 
+    widget->close();
+
     // Manually reset to break WebViewHelper's dependency on the stack allocated clients.
     reset();
 }
@@ -7842,15 +7860,18 @@ TEST_P(ParameterizedWebFrameTest, RemoteFrameInitialCommitType)
     // If an iframe has a remote main frame, ensure the inital commit is correctly identified as WebInitialCommitInChildFrame.
     CommitTypeWebFrameClient childFrameClient;
     WebLocalFrame* childFrame = FrameTestHelpers::createLocalChild(view->mainFrame()->toWebRemoteFrame(), "frameName", &childFrameClient);
+    FrameTestHelpers::TestWebWidgetClient widgetClient;
+    WebWidget* widget = WebFrameWidget::create(&widgetClient, childFrame);
     registerMockedHttpURLLoad("foo.html");
     FrameTestHelpers::loadFrame(childFrame, m_baseURL + "foo.html");
     EXPECT_EQ(WebInitialCommitInChildFrame, childFrameClient.historyCommitType());
+    widget->close();
     view->close();
 }
 
-class GestureEventTestWebViewClient : public FrameTestHelpers::TestWebViewClient {
+class GestureEventTestWebWidgetClient : public FrameTestHelpers::TestWebWidgetClient {
 public:
-    GestureEventTestWebViewClient() : m_didHandleGestureEvent(false) { }
+    GestureEventTestWebWidgetClient() : m_didHandleGestureEvent(false) { }
     void didHandleGestureEvent(const WebGestureEvent& event, bool eventCancelled) override { m_didHandleGestureEvent = true; }
     bool didHandleGestureEvent() const { return m_didHandleGestureEvent; }
 
@@ -7868,13 +7889,13 @@ TEST_P(ParameterizedWebFrameTest, FrameWidgetTest)
 
     WebLocalFrame* childFrame = FrameTestHelpers::createLocalChild(view->mainFrame()->toWebRemoteFrame());
 
-    GestureEventTestWebViewClient childViewClient;
-    WebFrameWidget* widget = WebFrameWidget::create(childViewClient.widgetClient(), childFrame);
+    GestureEventTestWebWidgetClient childWidgetClient;
+    WebFrameWidget* widget = WebFrameWidget::create(&childWidgetClient, childFrame);
 
     view->resize(WebSize(1000, 1000));
 
     widget->handleInputEvent(fatTap(20, 20));
-    EXPECT_TRUE(childViewClient.didHandleGestureEvent());
+    EXPECT_TRUE(childWidgetClient.didHandleGestureEvent());
 
     widget->close();
     view->close();
@@ -8098,6 +8119,8 @@ TEST_P(ParameterizedWebFrameTest, SendBeaconFromChildWithRemoteMainFrame)
     root->setReplicatedOrigin(SecurityOrigin::createUnique());
 
     WebLocalFrame* localFrame = FrameTestHelpers::createLocalChild(root);
+    FrameTestHelpers::TestWebWidgetClient widgetClient;
+    WebWidget* widget = WebFrameWidget::create(&widgetClient, localFrame);
 
     // Finally, make sure an embedder triggered load in the local frame swapped
     // back in works.
@@ -8105,6 +8128,7 @@ TEST_P(ParameterizedWebFrameTest, SendBeaconFromChildWithRemoteMainFrame)
     registerMockedHttpURLLoad("reload_post.html"); // url param to sendBeacon()
     FrameTestHelpers::loadFrame(localFrame, m_baseURL + "send_beacon.html");
 
+    widget->close();
     view->close();
 }
 
@@ -8123,6 +8147,8 @@ TEST_P(ParameterizedWebFrameTest, RemoteToLocalSwapOnMainFrameInitializesCoreFra
     // Do a remote-to-local swap of the top frame.
     FrameTestHelpers::TestWebFrameClient localClient;
     WebLocalFrame* localRoot = WebLocalFrame::createProvisional(&localClient, remoteRoot, WebSandboxFlags::None);
+    FrameTestHelpers::TestWebViewWidgetClient widgetClient(&viewClient);
+    WebWidget* widget = WebFrameWidget::create(&widgetClient, view, localRoot);
     remoteRoot->swap(localRoot);
 
     // Load a page with a child frame in the new root to make sure this doesn't
@@ -8131,6 +8157,7 @@ TEST_P(ParameterizedWebFrameTest, RemoteToLocalSwapOnMainFrameInitializesCoreFra
     registerMockedHttpURLLoad("visible_iframe.html");
     FrameTestHelpers::loadFrame(localRoot, m_baseURL + "single_iframe.html");
 
+    widget->close();
     view->close();
 }