Title: [185127] trunk/Source/WebKit2
Revision
185127
Author
barraclo...@apple.com
Date
2015-06-02 15:02:35 -0700 (Tue, 02 Jun 2015)

Log Message

PDFs always think they're visible on iOS.
https://bugs.webkit.org/show_bug.cgi?id=145493
<rdar://problem/19668879>

Reviewed by Andreas Kling & Sam Weinig.

The problem here is that WKContentView is currently responsible for notifying the WebPageProxy
that the visibility may have changed, but when a PDF isn't showing the WKContentView isn't in
the view hierarchy and doesn't receive the didMoveToWindow notifications – and the WKPDFView
(which is in the view hierarchy) does not listen for these events.

Visibility of the page should really just track the visibility of the WKWebView (and when
actually assessing the visibility it largely does - the page client checks the web view's
visibility, bar a FIXME, and the foreground/background check, which needs to change).
So notifications should really just come from the WKWebView.

The WKWebView already listens for the didMoveToWindow notification, it just was only updating
the IsInWindow flag. Instead just update all flags, and the call to viewStateDidChange from
WKContentView can just be removed.

There is one problem with this in that it would reverse the order of the calls to
viewStateDidChange & _updateForScreen:, which would mean the the view would become visible before
updating the screen pixel density. To fix this, moving the call to _updateForScreen:
to willMoveToWindow:, to ensure it occurs before the page becomes visible.

This will also change behavior on Mac for WKWebView clients, in coalescing all view state changes
within the didMoveToWindow call. This is the direction we intended to go in anyway (the plan is
to remove the mayHaveChanged argument from viewStateDidChange - we're currently adding unnecessary
IPC traffic).

* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView didMoveToWindow]):
    - should update all view state flags.
* UIProcess/API/ios/WKViewIOS.mm:
(-[WKView didMoveToWindow]):
    - added to match WKWebView.mm, for WebKitTestRunner.
* UIProcess/ios/WKContentView.mm:
(-[WKContentView willMoveToWindow:]):
    - should _updateForScreen:
(-[WKContentView didMoveToWindow]): Deleted.
    - functionality moved to -[WKWebView didMoveToWindow], -[WKContentView didMoveToWindow].

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (185126 => 185127)


--- trunk/Source/WebKit2/ChangeLog	2015-06-02 21:54:13 UTC (rev 185126)
+++ trunk/Source/WebKit2/ChangeLog	2015-06-02 22:02:35 UTC (rev 185127)
@@ -1,3 +1,47 @@
+2015-06-02  Gavin Barraclough  <barraclo...@apple.com>
+
+        PDFs always think they're visible on iOS.
+        https://bugs.webkit.org/show_bug.cgi?id=145493
+        <rdar://problem/19668879>
+
+        Reviewed by Andreas Kling & Sam Weinig.
+
+        The problem here is that WKContentView is currently responsible for notifying the WebPageProxy
+        that the visibility may have changed, but when a PDF isn't showing the WKContentView isn't in
+        the view hierarchy and doesn't receive the didMoveToWindow notifications – and the WKPDFView
+        (which is in the view hierarchy) does not listen for these events.
+
+        Visibility of the page should really just track the visibility of the WKWebView (and when
+        actually assessing the visibility it largely does - the page client checks the web view's
+        visibility, bar a FIXME, and the foreground/background check, which needs to change).
+        So notifications should really just come from the WKWebView.
+
+        The WKWebView already listens for the didMoveToWindow notification, it just was only updating
+        the IsInWindow flag. Instead just update all flags, and the call to viewStateDidChange from
+        WKContentView can just be removed.
+
+        There is one problem with this in that it would reverse the order of the calls to
+        viewStateDidChange & _updateForScreen:, which would mean the the view would become visible before
+        updating the screen pixel density. To fix this, moving the call to _updateForScreen:
+        to willMoveToWindow:, to ensure it occurs before the page becomes visible.
+
+        This will also change behavior on Mac for WKWebView clients, in coalescing all view state changes
+        within the didMoveToWindow call. This is the direction we intended to go in anyway (the plan is
+        to remove the mayHaveChanged argument from viewStateDidChange - we're currently adding unnecessary
+        IPC traffic).
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView didMoveToWindow]):
+            - should update all view state flags.
+        * UIProcess/API/ios/WKViewIOS.mm:
+        (-[WKView didMoveToWindow]):
+            - added to match WKWebView.mm, for WebKitTestRunner.
+        * UIProcess/ios/WKContentView.mm:
+        (-[WKContentView willMoveToWindow:]):
+            - should _updateForScreen:
+        (-[WKContentView didMoveToWindow]): Deleted.
+            - functionality moved to -[WKWebView didMoveToWindow], -[WKContentView didMoveToWindow].
+
 2015-06-02  Chris Dumez  <cdu...@apple.com>
 
         [iOS][WK2] Always mark layers as volatile for background pages

Modified: trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm (185126 => 185127)


--- trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm	2015-06-02 21:54:13 UTC (rev 185126)
+++ trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm	2015-06-02 22:02:35 UTC (rev 185127)
@@ -1320,7 +1320,7 @@
 
 - (void)didMoveToWindow
 {
-    _page->viewStateDidChange(WebCore::ViewState::IsInWindow);
+    _page->viewStateDidChange(WebCore::ViewState::AllFlags);
 }
 
 - (void)setOpaque:(BOOL)opaque

Modified: trunk/Source/WebKit2/UIProcess/API/ios/WKViewIOS.mm (185126 => 185127)


--- trunk/Source/WebKit2/UIProcess/API/ios/WKViewIOS.mm	2015-06-02 21:54:13 UTC (rev 185126)
+++ trunk/Source/WebKit2/UIProcess/API/ios/WKViewIOS.mm	2015-06-02 22:02:35 UTC (rev 185127)
@@ -113,6 +113,11 @@
         [self _frameOrBoundsChanged];
 }
 
+- (void)didMoveToWindow
+{
+    [_contentView page]->viewStateDidChange(WebCore::ViewState::AllFlags);
+}
+
 - (UIScrollView *)scrollView
 {
     return _scrollView.get();

Modified: trunk/Source/WebKit2/UIProcess/ios/WKContentView.mm (185126 => 185127)


--- trunk/Source/WebKit2/UIProcess/ios/WKContentView.mm	2015-06-02 21:54:13 UTC (rev 185126)
+++ trunk/Source/WebKit2/UIProcess/ios/WKContentView.mm	2015-06-02 22:02:35 UTC (rev 185127)
@@ -281,16 +281,11 @@
         if (_webView._allowsLinkPreview)
             [self _registerPreviewInWindow:newWindow];
 #endif
+
+        [self _updateForScreen:newWindow.screen];
     }
 }
 
-- (void)didMoveToWindow
-{
-    if (self.window)
-        [self _updateForScreen:self.window.screen];
-    _page->viewStateDidChange(ViewState::AllFlags);
-}
-
 - (WKBrowsingContextController *)browsingContextController
 {
     if (!_browsingContextController)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to