Bug 195037

Summary: [iOS] REGRESSION(r238490?): Safari sometimes shows blank page until a cross site navigation or re-opening the tab
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: Layout and RenderingAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, cdumez, commit-queue, ggaren, koivisto, simon.fraser, thorton, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Fix attempt
none
Patch for landing rniwa: commit-queue+

Description Ryosuke Niwa 2019-02-25 22:03:01 PST
Sometimes, Safari on iOS navigates to a blank page where the user can scroll, zoom, pan, etc... but nothing updates on the screen.

<rdar://problem/48154508>
Comment 1 Ryosuke Niwa 2019-02-25 22:41:41 PST
I was able to reproduce this issue on my phone other day, and upon detailed analysis, we came to conclusion that LayerTreeFreezeReason::ProcessSuspended is set in the affected WebPage, causing RemoteLayerTreeDrawingArea:: m_isFlushingSuspended to remain true, even though the page is clearly visible and I was actively interacting with it. This resulted in the layer tree to be never committed as RemoteLayerTreeDrawingArea::scheduleCompositingLayerFlush() would exit early in the first branch:

void RemoteLayerTreeDrawingArea::scheduleCompositingLayerFlush()
{
    if (m_isFlushingSuspended) {
        m_isLayerFlushThrottlingTemporarilyDisabledForInteraction = false;
        m_hasDeferredFlush = true;
        return;
    }
    if (m_isLayerFlushThrottlingTemporarilyDisabledForInteraction) {
        m_isLayerFlushThrottlingTemporarilyDisabledForInteraction = false;
        scheduleCompositingLayerFlushImmediately();
        return;
    }
...
}

It means that WebProcess::freezeAllLayerTrees() was called but WebProcess::unfreezeAllLayerTrees() was somehow never called in this web content process.

All evidence points to that ProcessThrottler in UI process is suspending the process but never waking up since all calls to WebProcess::freezeAllLayerTrees() and WebProcess::unfreezeAllLayerTrees() originate from ProcessThrottler. In fact, the console log on my phone indicated that the process was releasing the foreground assertion but never regaining its foreground status.

However, Antti pointed out to me that he recently refactored WebPage::freezeLayerTree to use a OptionSet instead of relying upon a boolean argument and some boolean states of WebPage in https://trac.webkit.org/changeset/238490. Prior to this change, WebProcess::unfreezeAllLayerTrees() was necessary as long as WebPage::didCompletePageTransition, WebPage::endPrinting, or WebPage::applicationWillEnterForeground was called. Because WebPage::didCompletePageTransition would happen on almost every page load, it's possible that prior to this code change, iOS Safari was suffering from the same problem but the symptom was never surfacing as a end-user visible bug because WebPage::didCompletePageTransition would eventually come around to fix it.
Comment 2 Ryosuke Niwa 2019-02-25 22:50:01 PST
Created attachment 362966 [details]
Fix attempt
Comment 3 Chris Dumez 2019-02-26 08:40:42 PST
Comment on attachment 362966 [details]
Fix attempt

View in context: https://bugs.webkit.org/attachment.cgi?id=362966&action=review

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:3106
> +    unfreezeLayerTree(LayerTreeFreezeReason::ProcessSuspended);

Could we RELEASE_LOG_ERROR() or ASSERT() if LayerTreeFreezeReason::ProcessSuspended is still set when WebPage::didCompletePageTransition() is called?
Comment 4 Antti Koivisto 2019-02-26 09:25:34 PST
Comment on attachment 362966 [details]
Fix attempt

r=me
Comment 5 Ryosuke Niwa 2019-02-26 11:52:07 PST
(In reply to Chris Dumez from comment #3)
> Comment on attachment 362966 [details]
> Fix attempt
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=362966&action=review
> 
> > Source/WebKit/WebProcess/WebPage/WebPage.cpp:3106
> > +    unfreezeLayerTree(LayerTreeFreezeReason::ProcessSuspended);
> 
> Could we RELEASE_LOG_ERROR() or ASSERT() if
> LayerTreeFreezeReason::ProcessSuspended is still set when
> WebPage::didCompletePageTransition() is called?

Sure, let's add RELEASE_LOG_ERROR.
Comment 6 Ryosuke Niwa 2019-02-26 12:09:10 PST
Created attachment 363002 [details]
Patch for landing
Comment 7 Ryosuke Niwa 2019-02-26 12:24:45 PST
Committed r242098: <https://trac.webkit.org/changeset/242098>
Comment 8 Radar WebKit Bug Importer 2019-02-26 12:31:24 PST
<rdar://problem/48410096>