REGRESSION (r233480): Mail contents flash black when activating
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 Jul 2018 06:22:14 +0000 (06:22 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 Jul 2018 06:22:14 +0000 (06:22 +0000)
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.

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/ApplicationStateTracker.h
Source/WebKit/UIProcess/ApplicationStateTracker.mm
Source/WebKit/UIProcess/ios/WKApplicationStateTrackingView.h
Source/WebKit/UIProcess/ios/WKApplicationStateTrackingView.mm
Source/WebKit/UIProcess/ios/WKContentView.mm
Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm

index 9231fa6..cf47ddf 100644 (file)
@@ -1,3 +1,74 @@
+2018-07-10  Tim Horton  <timothy_horton@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  <youenn@apple.com>
 
         Make fetch() use "same-origin" credentials by default
index 4eb5b2b..3eaa503 100644 (file)
@@ -39,20 +39,18 @@ namespace WebKit {
 
 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; }
 
 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 @@ private:
     RetainPtr<BKSApplicationStateMonitor> m_applicationStateMonitor;
 
     id m_didEnterBackgroundObserver;
-    id m_didCreateWindowContextObserver;
     id m_didFinishSnapshottingAfterEnteringBackgroundObserver;
     id m_willEnterForegroundObserver;
 };
index a2f864e..b878848 100644 (file)
@@ -73,20 +73,17 @@ static bool isBackgroundState(BKSApplicationState state)
     }
 }
 
-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 @@ ApplicationStateTracker::ApplicationStateTracker(UIView *view, SEL didEnterBackg
     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 @@ ApplicationStateTracker::~ApplicationStateTracker()
 
     NSNotificationCenter *notificationCenter = [NSNotificationCenter defaultCenter];
     [notificationCenter removeObserver:m_didEnterBackgroundObserver];
-    [notificationCenter removeObserver:m_didCreateWindowContextObserver];
     [notificationCenter removeObserver:m_didFinishSnapshottingAfterEnteringBackgroundObserver];
     [notificationCenter removeObserver:m_willEnterForegroundObserver];
 }
@@ -213,12 +202,6 @@ void ApplicationStateTracker::applicationDidEnterBackground()
         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())
index fd7e78a..3d180b4 100644 (file)
@@ -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;
index 037fdd3..884e81f 100644 (file)
@@ -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
     page->activityStateDidChange(WebCore::ActivityState::AllFlags & ~WebCore::ActivityState::IsInWindow);
 }
 
-- (void)_applicationDidCreateWindowContext
-{
-}
-
 - (void)_applicationDidFinishSnapshottingAfterEnteringBackground
 {
     if (auto page = [_webView _page])
index 669b988..5f616b5 100644 (file)
@@ -621,13 +621,6 @@ static void storeAccessibilityRemoteConnectionInformation(id element, pid_t pid,
     _page->applicationWillResignActive();
 }
 
-- (void)_applicationDidCreateWindowContext
-{
-    [super _applicationDidCreateWindowContext];
-    if (auto drawingArea = _page->drawingArea())
-        drawingArea->hideContentUntilAnyUpdate();
-}
-
 - (void)_applicationDidBecomeActive:(NSNotification*)notification
 {
     _page->applicationDidBecomeActive();
index 7a704c1..ffbd8e1 100644 (file)
@@ -653,8 +653,10 @@ void WebPageProxy::applicationDidEnterBackground()
 
 void WebPageProxy::applicationDidFinishSnapshottingAfterEnteringBackground()
 {
-    if (m_drawingArea)
+    if (m_drawingArea) {
         m_drawingArea->prepareForAppSuspension();
+        m_drawingArea->hideContentUntilAnyUpdate();
+    }
     m_process->send(Messages::WebPage::ApplicationDidFinishSnapshottingAfterEnteringBackground(), m_pageID);
 }