WebPageCreationParameters should be consistent in Window.open
authorbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 31 Oct 2013 17:48:33 +0000 (17:48 +0000)
committerbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 31 Oct 2013 17:48:33 +0000 (17:48 +0000)
https://bugs.webkit.org/show_bug.cgi?id=123557

Reviewed by Sam Weinig.

When Window.open in called in WebKit2 the WebProcess sends a synchronous request to open
a page (WebPageProxy::createNewPage). The UIProcess creates a WebpageProxy, and responds
with instructions to the WebProcess to create the WebPage. The initial creation state of
the WebPage is communicated to the WebProcess via two routes (firstly an asynchronous
WebProcess::CreateWebPage message, and secondly in the synchronous response from
WebPageProxy::createNewPage). Unfortunately these responses are inconsistent with each
other. The creationParameters() for the page are calculated twice, and since the WKView
will be added to a window between the async message being sent and the synchronous reply
being returned the visibility state of the page can change.

To fix the inconsistency we can set the creation parameters at the point that the
WebPageProxy is instantiated. This will result in a functional change that is web
visible, since the page will initially be opened in a hidden/blurred state, and may
later become visible/focussed. This change is consistent with the direction we want to
evolve in. Whilst we will still probably require a synchronous message from the
WebProcess to the UIProcess on Window.open, we'll probably make this return much earlier
– having just created the WebPageProxy, but avoiding blocking the WebProcess over the
client delegate callback that opens the new window.

This fix results in a layout test result change, due to the change in behavior (page is
created blurred, and becomes focussed, resulting in a focus event being fired – rather
than the window opening directly into a focussed state). This is reported as a
progression (test is broken in WebKit1, fixed in WebKit2 after this change). In reality
the test is actually slightly broken in DRT/test-runner – the test runs differently than
in browser, since there is is no visible main window. In-browser this patch results in
no change in behavior on dom/Window/mozilla-focus-blur.html (the affected test).

Source/WebKit2:

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::initializeWebPage):
    - call initializeCreationParameters
(WebKit::WebPageProxy::initializeCreationParameters):
    - calculate m_creationParameters
* UIProcess/WebPageProxy.h:
(WebKit::WebPageProxy::creationParameters):
    - Added m_creationParameters, initializeCreationParameters,
      creationParameters returns m_creationParameters

LayoutTests:

* platform/mac-wk2/fast/dom/Window: Added.
* platform/mac-wk2/fast/dom/Window/mozilla-focus-blur-expected.txt: Added.
    - This test now reports a pass on WebKit2

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@158369 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/platform/mac-wk2/fast/dom/Window/mozilla-focus-blur-expected.txt [new file with mode: 0644]
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/WebPageProxy.cpp
Source/WebKit2/UIProcess/WebPageProxy.h

index 36de5f6a5d57fed75ce9d67ebe756ca2b357bf8e..2bf927792b6982c719f07205c1406f285c25876b 100644 (file)
@@ -1,3 +1,41 @@
+2013-10-30  Gavin Barraclough  <barraclough@apple.com>
+
+        WebPageCreationParameters should be consistent in Window.open
+        https://bugs.webkit.org/show_bug.cgi?id=123557
+
+        Reviewed by Sam Weinig.
+
+        When Window.open in called in WebKit2 the WebProcess sends a synchronous request to open
+        a page (WebPageProxy::createNewPage). The UIProcess creates a WebpageProxy, and responds
+        with instructions to the WebProcess to create the WebPage. The initial creation state of
+        the WebPage is communicated to the WebProcess via two routes (firstly an asynchronous
+        WebProcess::CreateWebPage message, and secondly in the synchronous response from
+        WebPageProxy::createNewPage). Unfortunately these responses are inconsistent with each
+        other. The creationParameters() for the page are calculated twice, and since the WKView
+        will be added to a window between the async message being sent and the synchronous reply
+        being returned the visibility state of the page can change.
+
+        To fix the inconsistency we can set the creation parameters at the point that the
+        WebPageProxy is instantiated. This will result in a functional change that is web
+        visible, since the page will initially be opened in a hidden/blurred state, and may
+        later become visible/focussed. This change is consistent with the direction we want to
+        evolve in. Whilst we will still probably require a synchronous message from the
+        WebProcess to the UIProcess on Window.open, we'll probably make this return much earlier
+        – having just created the WebPageProxy, but avoiding blocking the WebProcess over the
+        client delegate callback that opens the new window.
+
+        This fix results in a layout test result change, due to the change in behavior (page is
+        created blurred, and becomes focussed, resulting in a focus event being fired – rather
+        than the window opening directly into a focussed state). This is reported as a
+        progression (test is broken in WebKit1, fixed in WebKit2 after this change). In reality
+        the test is actually slightly broken in DRT/test-runner – the test runs differently than
+        in browser, since there is is no visible main window. In-browser this patch results in
+        no change in behavior on dom/Window/mozilla-focus-blur.html (the affected test).
+
+        * platform/mac-wk2/fast/dom/Window: Added.
+        * platform/mac-wk2/fast/dom/Window/mozilla-focus-blur-expected.txt: Added.
+            - This test now reports a pass on WebKit2
+
 2013-10-31  Alexey Proskuryakov  <ap@apple.com>
 
         REGRESSION(r158333): http/tests/xmlhttprequest/response-encoding.html and xmlhttprequest-overridemimetype-content-type-header.html are failing
diff --git a/LayoutTests/platform/mac-wk2/fast/dom/Window/mozilla-focus-blur-expected.txt b/LayoutTests/platform/mac-wk2/fast/dom/Window/mozilla-focus-blur-expected.txt
new file mode 100644 (file)
index 0000000..a59e640
--- /dev/null
@@ -0,0 +1,11 @@
+Check that window.blur() does nothing, and window.focus() only works if it is invoked from the window that opened the former. If the test passes, you should see a series of PASS messages with the last being 'All tests finished'.
+
+This test is adopted from mozilla's tests.
+
+PASS: The focus should not have been changed!
+PASS: The focus should not have been changed!
+PASS: The focus should not have been changed with URL=data:text/html,<script>opener.focus();opener.postMessage("", "*");</script>
+PASS: The focus should not have been changed with URL=data:text/html,<script>blur();opener.postMessage("", "*");</script>
+PASS: The last opened window should be able to get focus
+PASS: All tests finished
+
index e25cf6a4fdc0269bac8546988b47d647d404a718..85cbc4344c46abc6ecc63a6ef715f8b59835edcb 100644 (file)
@@ -1,3 +1,47 @@
+2013-10-30  Gavin Barraclough  <barraclough@apple.com>
+
+        WebPageCreationParameters should be consistent in Window.open
+        https://bugs.webkit.org/show_bug.cgi?id=123557
+
+        Reviewed by Sam Weinig.
+
+        When Window.open in called in WebKit2 the WebProcess sends a synchronous request to open
+        a page (WebPageProxy::createNewPage). The UIProcess creates a WebpageProxy, and responds
+        with instructions to the WebProcess to create the WebPage. The initial creation state of
+        the WebPage is communicated to the WebProcess via two routes (firstly an asynchronous
+        WebProcess::CreateWebPage message, and secondly in the synchronous response from
+        WebPageProxy::createNewPage). Unfortunately these responses are inconsistent with each
+        other. The creationParameters() for the page are calculated twice, and since the WKView
+        will be added to a window between the async message being sent and the synchronous reply
+        being returned the visibility state of the page can change.
+
+        To fix the inconsistency we can set the creation parameters at the point that the
+        WebPageProxy is instantiated. This will result in a functional change that is web
+        visible, since the page will initially be opened in a hidden/blurred state, and may
+        later become visible/focussed. This change is consistent with the direction we want to
+        evolve in. Whilst we will still probably require a synchronous message from the
+        WebProcess to the UIProcess on Window.open, we'll probably make this return much earlier
+        – having just created the WebPageProxy, but avoiding blocking the WebProcess over the
+        client delegate callback that opens the new window.
+
+        This fix results in a layout test result change, due to the change in behavior (page is
+        created blurred, and becomes focussed, resulting in a focus event being fired – rather
+        than the window opening directly into a focussed state). This is reported as a
+        progression (test is broken in WebKit1, fixed in WebKit2 after this change). In reality
+        the test is actually slightly broken in DRT/test-runner – the test runs differently than
+        in browser, since there is is no visible main window. In-browser this patch results in
+        no change in behavior on dom/Window/mozilla-focus-blur.html (the affected test).
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::initializeWebPage):
+            - call initializeCreationParameters
+        (WebKit::WebPageProxy::initializeCreationParameters):
+            - calculate m_creationParameters
+        * UIProcess/WebPageProxy.h:
+        (WebKit::WebPageProxy::creationParameters):
+            - Added m_creationParameters, initializeCreationParameters,
+              creationParameters returns m_creationParameters
+
 2013-10-31  Philippe Normand  <pnormand@igalia.com>
 
         [WK2][GTK] enable-media-stream Setting
index 2dc1862ba291765d927bac1f129a81c72ea6c941..1d39fec7be31870ebdee3dfca600e876963d6141 100644 (file)
@@ -522,7 +522,8 @@ void WebPageProxy::initializeWebPage()
         inspector()->enableRemoteInspection();
 #endif
 
-    m_process->send(Messages::WebProcess::CreateWebPage(m_pageID, creationParameters()), 0);
+    initializeCreationParameters();
+    m_process->send(Messages::WebProcess::CreateWebPage(m_pageID, m_creationParameters), 0);
 
 #if ENABLE(PAGE_VISIBILITY_API)
     m_process->send(Messages::WebPage::SetVisibilityState(m_visibilityState, /* isInitialState */ true), m_pageID);
@@ -3832,47 +3833,43 @@ void WebPageProxy::resetStateAfterProcessExited()
 #endif
 }
 
-WebPageCreationParameters WebPageProxy::creationParameters() const
-{
-    WebPageCreationParameters parameters;
-
-    parameters.viewSize = m_pageClient->viewSize();
-    parameters.isActive = m_pageClient->isViewWindowActive();
-    parameters.isFocused = m_pageClient->isViewFocused();
-    parameters.isVisible = m_pageClient->isViewVisible();
-    parameters.isInWindow = m_pageClient->isViewInWindow();
-    parameters.drawingAreaType = m_drawingArea->type();
-    parameters.store = m_pageGroup->preferences()->store();
-    parameters.pageGroupData = m_pageGroup->data();
-    parameters.drawsBackground = m_drawsBackground;
-    parameters.drawsTransparentBackground = m_drawsTransparentBackground;
-    parameters.underlayColor = m_underlayColor;
-    parameters.areMemoryCacheClientCallsEnabled = m_areMemoryCacheClientCallsEnabled;
-    parameters.useFixedLayout = m_useFixedLayout;
-    parameters.fixedLayoutSize = m_fixedLayoutSize;
-    parameters.suppressScrollbarAnimations = m_suppressScrollbarAnimations;
-    parameters.paginationMode = m_paginationMode;
-    parameters.paginationBehavesLikeColumns = m_paginationBehavesLikeColumns;
-    parameters.pageLength = m_pageLength;
-    parameters.gapBetweenPages = m_gapBetweenPages;
-    parameters.userAgent = userAgent();
-    parameters.sessionState = SessionState(m_backForwardList->entries(), m_backForwardList->currentIndex());
-    parameters.highestUsedBackForwardItemID = WebBackForwardListItem::highedUsedItemID();
-    parameters.canRunBeforeUnloadConfirmPanel = m_uiClient.canRunBeforeUnloadConfirmPanel();
-    parameters.canRunModal = m_canRunModal;
-    parameters.deviceScaleFactor = deviceScaleFactor();
-    parameters.mediaVolume = m_mediaVolume;
-    parameters.mayStartMediaWhenInWindow = m_mayStartMediaWhenInWindow;
-    parameters.minimumLayoutSize = m_minimumLayoutSize;
-    parameters.autoSizingShouldExpandToViewHeight = m_autoSizingShouldExpandToViewHeight;
-    parameters.scrollPinningBehavior = m_scrollPinningBehavior;
+void WebPageProxy::initializeCreationParameters()
+{
+    m_creationParameters.viewSize = m_pageClient->viewSize();
+    m_creationParameters.isActive = m_pageClient->isViewWindowActive();
+    m_creationParameters.isFocused = m_pageClient->isViewFocused();
+    m_creationParameters.isVisible = m_pageClient->isViewVisible();
+    m_creationParameters.isInWindow = m_pageClient->isViewInWindow();
+    m_creationParameters.drawingAreaType = m_drawingArea->type();
+    m_creationParameters.store = m_pageGroup->preferences()->store();
+    m_creationParameters.pageGroupData = m_pageGroup->data();
+    m_creationParameters.drawsBackground = m_drawsBackground;
+    m_creationParameters.drawsTransparentBackground = m_drawsTransparentBackground;
+    m_creationParameters.underlayColor = m_underlayColor;
+    m_creationParameters.areMemoryCacheClientCallsEnabled = m_areMemoryCacheClientCallsEnabled;
+    m_creationParameters.useFixedLayout = m_useFixedLayout;
+    m_creationParameters.fixedLayoutSize = m_fixedLayoutSize;
+    m_creationParameters.suppressScrollbarAnimations = m_suppressScrollbarAnimations;
+    m_creationParameters.paginationMode = m_paginationMode;
+    m_creationParameters.paginationBehavesLikeColumns = m_paginationBehavesLikeColumns;
+    m_creationParameters.pageLength = m_pageLength;
+    m_creationParameters.gapBetweenPages = m_gapBetweenPages;
+    m_creationParameters.userAgent = userAgent();
+    m_creationParameters.sessionState = SessionState(m_backForwardList->entries(), m_backForwardList->currentIndex());
+    m_creationParameters.highestUsedBackForwardItemID = WebBackForwardListItem::highedUsedItemID();
+    m_creationParameters.canRunBeforeUnloadConfirmPanel = m_uiClient.canRunBeforeUnloadConfirmPanel();
+    m_creationParameters.canRunModal = m_canRunModal;
+    m_creationParameters.deviceScaleFactor = deviceScaleFactor();
+    m_creationParameters.mediaVolume = m_mediaVolume;
+    m_creationParameters.mayStartMediaWhenInWindow = m_mayStartMediaWhenInWindow;
+    m_creationParameters.minimumLayoutSize = m_minimumLayoutSize;
+    m_creationParameters.autoSizingShouldExpandToViewHeight = m_autoSizingShouldExpandToViewHeight;
+    m_creationParameters.scrollPinningBehavior = m_scrollPinningBehavior;
 
 #if PLATFORM(MAC)
-    parameters.layerHostingMode = m_layerHostingMode;
-    parameters.colorSpace = m_pageClient->colorSpace();
+    m_creationParameters.layerHostingMode = m_layerHostingMode;
+    m_creationParameters.colorSpace = m_pageClient->colorSpace();
 #endif
-
-    return parameters;
 }
 
 #if USE(ACCELERATED_COMPOSITING)
index fe7fcff86fc9f38b25786a50e7b1415f454ed6c6..54fbed3eb5f883f1a3e5109e8e6123accb8bdbf4 100644 (file)
@@ -50,6 +50,7 @@
 #include "WebHitTestResult.h"
 #include "WebLoaderClient.h"
 #include "WebPageContextMenuClient.h"
+#include "WebPageCreationParameters.h"
 #include <WebCore/AlternativeTextClient.h> // FIXME: Needed by WebPageProxyMessages.h for DICTATION_ALTERNATIVES.
 #include "WebPageProxyMessages.h"
 #include "WebPolicyClient.h"
@@ -148,7 +149,6 @@ struct DictionaryPopupInfo;
 struct EditorState;
 struct PlatformPopupMenuData;
 struct PrintInfo;
-struct WebPageCreationParameters;
 struct WebPopupItem;
 
 #if ENABLE(VIBRATION)
@@ -628,7 +628,10 @@ public:
     void didChooseFilesForOpenPanel(const Vector<String>&);
     void didCancelForOpenPanel();
 
-    WebPageCreationParameters creationParameters() const;
+    WebPageCreationParameters creationParameters() const
+    {
+        return m_creationParameters;
+    }
 
 #if USE(COORDINATED_GRAPHICS)
     void findZoomableAreaForPoint(const WebCore::IntPoint&, const WebCore::IntSize&);
@@ -751,6 +754,7 @@ public:
 private:
     WebPageProxy(PageClient*, PassRefPtr<WebProcessProxy>, WebPageGroup*, uint64_t pageID);
     void platformInitialize();
+    void initializeCreationParameters();
 
     void resetState();
     void resetStateAfterProcessExited();
@@ -1246,6 +1250,8 @@ private:
 #endif
         
     WebCore::ScrollPinningBehavior m_scrollPinningBehavior;
+
+    WebPageCreationParameters m_creationParameters;
 };
 
 } // namespace WebKit