Title: [233723] trunk/Source/WebKit
Revision
233723
Author
timothy_hor...@apple.com
Date
2018-07-10 23:22:14 -0700 (Tue, 10 Jul 2018)

Log Message

REGRESSION (r233480): Mail contents flash black when activating
https://bugs.webkit.org/show_bug.cgi?id=187504
<rdar://problem/41752351>

Reviewed by Simon Fraser.

The sequence of events to reproduce the bug originally fixed in r203371
is either:

A) the simple background/foreground case

1] app begins to suspend
2] app suspension snapshots are taken
3] WKWebView's surfaces are marked volatile
4] app completes suspension
    ... time goes by ...
5] WKWebView's volatile surfaces are purged
    ... time goes by ...
6] app begins to resume, shows (good) suspension snapshot
7] app removes suspension snapshot
8] WKWebView has sublayers with purged (black) surfaces
9] WKWebView sublayers are repaired by a new commit with nonvolatile surfaces

B) the re-snapshot while in the background case

1] app begins to suspend
2] app suspension snapshots are taken
3] WKWebView's surfaces are marked volatile
4] app completes suspension
... time goes by ...
5] WKWebView's volatile surfaces are purged
... time goes by ...
6] app wakes up in the background to update its snapshots
7] in the updated snapshots, WKWebView has sublayers with purged (black) surfaces
... time goes by ...
8] app begins to resume, shows (bad) suspension snapshot
9] WKWebView presents layers with purged (black) surfaces until new commit fixes them
10] WKWebView sublayers are repaired by a new commit with nonvolatile surfaces

WebKit's current approach to fix this problem is simply to hide the
WKWebView's sublayers at some point after A2/B2 (suspension snapshots),
but before A8/B7 (the first time the empty layers would be presented
or snapshotted).

Previously, we did this by hiding the layers when the window's CAContext
was created, which happened early enough in both cases (at A6/B6).
However, that notification was removed underneath us at some point.

However, in looking at the timelines, there's a better place to do this:
immediately after marking the surfaces volatile (A3/B3), which is always
strictly after the app suspension snapshots are taken, and also always
before the freshly-made-volatile layers could be presented or snapshotted.

* UIProcess/ApplicationStateTracker.h:
* UIProcess/ApplicationStateTracker.mm:
(WebKit::ApplicationStateTracker::ApplicationStateTracker):
(WebKit::ApplicationStateTracker::~ApplicationStateTracker):
(WebKit::ApplicationStateTracker::applicationDidCreateWindowContext): Deleted.
* UIProcess/ios/WKApplicationStateTrackingView.h:
* UIProcess/ios/WKApplicationStateTrackingView.mm:
(-[WKApplicationStateTrackingView didMoveToWindow]):
(-[WKApplicationStateTrackingView _applicationDidCreateWindowContext]): Deleted.
* UIProcess/ios/WKContentView.mm:
(-[WKContentView _applicationDidCreateWindowContext]): Deleted.
* UIProcess/ios/WebPageProxyIOS.mm:
(WebKit::WebPageProxy::applicationDidFinishSnapshottingAfterEnteringBackground):
Remove the didCreateWindowContext notification, and hide content after
snapshotting after entering the background.

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (233722 => 233723)


--- trunk/Source/WebKit/ChangeLog	2018-07-11 06:21:22 UTC (rev 233722)
+++ trunk/Source/WebKit/ChangeLog	2018-07-11 06:22:14 UTC (rev 233723)
@@ -1,3 +1,74 @@
+2018-07-10  Tim Horton  <timothy_hor...@apple.com>
+
+        REGRESSION (r233480): Mail contents flash black when activating
+        https://bugs.webkit.org/show_bug.cgi?id=187504
+        <rdar://problem/41752351>
+
+        Reviewed by Simon Fraser.
+
+        The sequence of events to reproduce the bug originally fixed in r203371
+        is either:
+
+        A) the simple background/foreground case
+
+        1] app begins to suspend
+        2] app suspension snapshots are taken
+        3] WKWebView's surfaces are marked volatile
+        4] app completes suspension
+            ... time goes by ...
+        5] WKWebView's volatile surfaces are purged
+            ... time goes by ...
+        6] app begins to resume, shows (good) suspension snapshot
+        7] app removes suspension snapshot
+        8] WKWebView has sublayers with purged (black) surfaces
+        9] WKWebView sublayers are repaired by a new commit with nonvolatile surfaces
+
+        B) the re-snapshot while in the background case
+
+        1] app begins to suspend
+        2] app suspension snapshots are taken
+        3] WKWebView's surfaces are marked volatile
+        4] app completes suspension
+        ... time goes by ...
+        5] WKWebView's volatile surfaces are purged
+        ... time goes by ...
+        6] app wakes up in the background to update its snapshots
+        7] in the updated snapshots, WKWebView has sublayers with purged (black) surfaces
+        ... time goes by ...
+        8] app begins to resume, shows (bad) suspension snapshot
+        9] WKWebView presents layers with purged (black) surfaces until new commit fixes them
+        10] WKWebView sublayers are repaired by a new commit with nonvolatile surfaces
+
+        WebKit's current approach to fix this problem is simply to hide the
+        WKWebView's sublayers at some point after A2/B2 (suspension snapshots),
+        but before A8/B7 (the first time the empty layers would be presented
+        or snapshotted).
+
+        Previously, we did this by hiding the layers when the window's CAContext
+        was created, which happened early enough in both cases (at A6/B6).
+        However, that notification was removed underneath us at some point.
+
+        However, in looking at the timelines, there's a better place to do this:
+        immediately after marking the surfaces volatile (A3/B3), which is always
+        strictly after the app suspension snapshots are taken, and also always
+        before the freshly-made-volatile layers could be presented or snapshotted.
+
+        * UIProcess/ApplicationStateTracker.h:
+        * UIProcess/ApplicationStateTracker.mm:
+        (WebKit::ApplicationStateTracker::ApplicationStateTracker):
+        (WebKit::ApplicationStateTracker::~ApplicationStateTracker):
+        (WebKit::ApplicationStateTracker::applicationDidCreateWindowContext): Deleted.
+        * UIProcess/ios/WKApplicationStateTrackingView.h:
+        * UIProcess/ios/WKApplicationStateTrackingView.mm:
+        (-[WKApplicationStateTrackingView didMoveToWindow]):
+        (-[WKApplicationStateTrackingView _applicationDidCreateWindowContext]): Deleted.
+        * UIProcess/ios/WKContentView.mm:
+        (-[WKContentView _applicationDidCreateWindowContext]): Deleted.
+        * UIProcess/ios/WebPageProxyIOS.mm:
+        (WebKit::WebPageProxy::applicationDidFinishSnapshottingAfterEnteringBackground):
+        Remove the didCreateWindowContext notification, and hide content after
+        snapshotting after entering the background.
+
 2018-07-10  Youenn Fablet  <you...@apple.com>
 
         Make fetch() use "same-origin" credentials by default

Modified: trunk/Source/WebKit/UIProcess/ApplicationStateTracker.h (233722 => 233723)


--- trunk/Source/WebKit/UIProcess/ApplicationStateTracker.h	2018-07-11 06:21:22 UTC (rev 233722)
+++ trunk/Source/WebKit/UIProcess/ApplicationStateTracker.h	2018-07-11 06:22:14 UTC (rev 233723)
@@ -39,7 +39,7 @@
 
 class ApplicationStateTracker : public CanMakeWeakPtr<ApplicationStateTracker> {
 public:
-    ApplicationStateTracker(UIView *, SEL didEnterBackgroundSelector, SEL didCreateWindowContextSelector, SEL didFinishSnapshottingAfterEnteringBackgroundSelector, SEL willEnterForegroundSelector);
+    ApplicationStateTracker(UIView *, SEL didEnterBackgroundSelector, SEL didFinishSnapshottingAfterEnteringBackgroundSelector, SEL willEnterForegroundSelector);
     ~ApplicationStateTracker();
 
     bool isInBackground() const { return m_isInBackground; }
@@ -46,13 +46,11 @@
 
 private:
     void applicationDidEnterBackground();
-    void applicationDidCreateWindowContext();
     void applicationDidFinishSnapshottingAfterEnteringBackground();
     void applicationWillEnterForeground();
 
     WeakObjCPtr<UIView> m_view;
     SEL m_didEnterBackgroundSelector;
-    SEL m_didCreateWindowContextSelector;
     SEL m_didFinishSnapshottingAfterEnteringBackgroundSelector;
     SEL m_willEnterForegroundSelector;
 
@@ -61,7 +59,6 @@
     RetainPtr<BKSApplicationStateMonitor> m_applicationStateMonitor;
 
     id m_didEnterBackgroundObserver;
-    id m_didCreateWindowContextObserver;
     id m_didFinishSnapshottingAfterEnteringBackgroundObserver;
     id m_willEnterForegroundObserver;
 };

Modified: trunk/Source/WebKit/UIProcess/ApplicationStateTracker.mm (233722 => 233723)


--- trunk/Source/WebKit/UIProcess/ApplicationStateTracker.mm	2018-07-11 06:21:22 UTC (rev 233722)
+++ trunk/Source/WebKit/UIProcess/ApplicationStateTracker.mm	2018-07-11 06:22:14 UTC (rev 233723)
@@ -73,20 +73,17 @@
     }
 }
 
-ApplicationStateTracker::ApplicationStateTracker(UIView *view, SEL didEnterBackgroundSelector, SEL didCreateWindowContextSelector, SEL didFinishSnapshottingAfterEnteringBackgroundSelector, SEL willEnterForegroundSelector)
+ApplicationStateTracker::ApplicationStateTracker(UIView *view, SEL didEnterBackgroundSelector, SEL didFinishSnapshottingAfterEnteringBackgroundSelector, SEL willEnterForegroundSelector)
     : m_view(view)
     , m_didEnterBackgroundSelector(didEnterBackgroundSelector)
-    , m_didCreateWindowContextSelector(didCreateWindowContextSelector)
     , m_didFinishSnapshottingAfterEnteringBackgroundSelector(didFinishSnapshottingAfterEnteringBackgroundSelector)
     , m_willEnterForegroundSelector(willEnterForegroundSelector)
     , m_isInBackground(true)
     , m_didEnterBackgroundObserver(nullptr)
-    , m_didCreateWindowContextObserver(nullptr)
     , m_didFinishSnapshottingAfterEnteringBackgroundObserver(nullptr)
     , m_willEnterForegroundObserver(nullptr)
 {
     ASSERT([m_view.get() respondsToSelector:m_didEnterBackgroundSelector]);
-    ASSERT([m_view.get() respondsToSelector:m_didCreateWindowContextSelector]);
     ASSERT([m_view.get() respondsToSelector:m_didFinishSnapshottingAfterEnteringBackgroundSelector]);
     ASSERT([m_view.get() respondsToSelector:m_willEnterForegroundSelector]);
 
@@ -97,13 +94,6 @@
     UIApplication *application = [UIApplication sharedApplication];
 
     auto weakThis = makeWeakPtr(*this);
-    // FIXME: this notification never fires any more: rdar://problem/41752351.
-    m_didCreateWindowContextObserver = [notificationCenter addObserverForName:@"_UIWindowDidCreateWindowContextNotification" object:window queue:nil usingBlock:[weakThis](NSNotification *) {
-        auto applicationStateTracker = weakThis.get();
-        if (!applicationStateTracker)
-            return;
-        applicationStateTracker->applicationDidCreateWindowContext();
-    }];
 
     m_didFinishSnapshottingAfterEnteringBackgroundObserver = [notificationCenter addObserverForName:@"_UIApplicationDidFinishSuspensionSnapshotNotification" object:application queue:nil usingBlock:[weakThis](NSNotification *) {
         auto applicationStateTracker = weakThis.get();
@@ -200,7 +190,6 @@
 
     NSNotificationCenter *notificationCenter = [NSNotificationCenter defaultCenter];
     [notificationCenter removeObserver:m_didEnterBackgroundObserver];
-    [notificationCenter removeObserver:m_didCreateWindowContextObserver];
     [notificationCenter removeObserver:m_didFinishSnapshottingAfterEnteringBackgroundObserver];
     [notificationCenter removeObserver:m_willEnterForegroundObserver];
 }
@@ -213,12 +202,6 @@
         wtfObjcMsgSend<void>(view.get(), m_didEnterBackgroundSelector);
 }
 
-void ApplicationStateTracker::applicationDidCreateWindowContext()
-{
-    if (auto view = m_view.get())
-        wtfObjcMsgSend<void>(view.get(), m_didCreateWindowContextSelector);
-}
-
 void ApplicationStateTracker::applicationDidFinishSnapshottingAfterEnteringBackground()
 {
     if (auto view = m_view.get())

Modified: trunk/Source/WebKit/UIProcess/ios/WKApplicationStateTrackingView.h (233722 => 233723)


--- trunk/Source/WebKit/UIProcess/ios/WKApplicationStateTrackingView.h	2018-07-11 06:21:22 UTC (rev 233722)
+++ trunk/Source/WebKit/UIProcess/ios/WKApplicationStateTrackingView.h	2018-07-11 06:22:14 UTC (rev 233723)
@@ -32,7 +32,6 @@
 @interface WKApplicationStateTrackingView : UIView
 
 - (instancetype)initWithFrame:(CGRect)frame webView:(WKWebView *)webView;
-- (void)_applicationDidCreateWindowContext;
 - (void)_applicationWillEnterForeground;
 @property (nonatomic, readonly) BOOL isBackground;
 @property (nonatomic, readonly) UIView *_contentView;

Modified: trunk/Source/WebKit/UIProcess/ios/WKApplicationStateTrackingView.mm (233722 => 233723)


--- trunk/Source/WebKit/UIProcess/ios/WKApplicationStateTrackingView.mm	2018-07-11 06:21:22 UTC (rev 233722)
+++ trunk/Source/WebKit/UIProcess/ios/WKApplicationStateTrackingView.mm	2018-07-11 06:22:14 UTC (rev 233723)
@@ -63,7 +63,7 @@
         return;
 
     ASSERT(!_applicationStateTracker);
-    _applicationStateTracker = std::make_unique<WebKit::ApplicationStateTracker>(self, @selector(_applicationDidEnterBackground), @selector(_applicationDidCreateWindowContext), @selector(_applicationDidFinishSnapshottingAfterEnteringBackground), @selector(_applicationWillEnterForeground));
+    _applicationStateTracker = std::make_unique<WebKit::ApplicationStateTracker>(self, @selector(_applicationDidEnterBackground), @selector(_applicationDidFinishSnapshottingAfterEnteringBackground), @selector(_applicationWillEnterForeground));
 }
 
 - (void)_applicationDidEnterBackground
@@ -76,10 +76,6 @@
     page->activityStateDidChange(WebCore::ActivityState::AllFlags & ~WebCore::ActivityState::IsInWindow);
 }
 
-- (void)_applicationDidCreateWindowContext
-{
-}
-
 - (void)_applicationDidFinishSnapshottingAfterEnteringBackground
 {
     if (auto page = [_webView _page])

Modified: trunk/Source/WebKit/UIProcess/ios/WKContentView.mm (233722 => 233723)


--- trunk/Source/WebKit/UIProcess/ios/WKContentView.mm	2018-07-11 06:21:22 UTC (rev 233722)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentView.mm	2018-07-11 06:22:14 UTC (rev 233723)
@@ -621,13 +621,6 @@
     _page->applicationWillResignActive();
 }
 
-- (void)_applicationDidCreateWindowContext
-{
-    [super _applicationDidCreateWindowContext];
-    if (auto drawingArea = _page->drawingArea())
-        drawingArea->hideContentUntilAnyUpdate();
-}
-
 - (void)_applicationDidBecomeActive:(NSNotification*)notification
 {
     _page->applicationDidBecomeActive();

Modified: trunk/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm (233722 => 233723)


--- trunk/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm	2018-07-11 06:21:22 UTC (rev 233722)
+++ trunk/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm	2018-07-11 06:22:14 UTC (rev 233723)
@@ -653,8 +653,10 @@
 
 void WebPageProxy::applicationDidFinishSnapshottingAfterEnteringBackground()
 {
-    if (m_drawingArea)
+    if (m_drawingArea) {
         m_drawingArea->prepareForAppSuspension();
+        m_drawingArea->hideContentUntilAnyUpdate();
+    }
     m_process->send(Messages::WebPage::ApplicationDidFinishSnapshottingAfterEnteringBackground(), m_pageID);
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to