Changeset 247195 in webkit

Timestamp:

Jul 6, 2019 5:20:03 PM (5 years ago)

Author:

Simon Fraser

Message:

Long hang when loading a cnn.com page on iOS
​https://bugs.webkit.org/show_bug.cgi?id=199556

Reviewed by Zalan Bujtas.
Source/WebCore:

Loading ​https://edition.cnn.com/travel/article/brussels-airlines-flight-to-nowhere/index.html in the iOS 13 sim
results in a long hang under OverlapMapContainer::append(). We were creating pathological clipping scopes with
thousands of entries, because OverlapMapContainer::mergeClippingScopesRecursive() had a logic error where
it added 'sourceScope' to the child instead of 'sourceChildScope'. Add a new assertion to detect that case.

I wasn't able to create a testcase that caused a hang, but a number of existing tests would have
hit the assertion.

• rendering/LayerOverlapMap.cpp:

(WebCore::OverlapMapContainer::ClippingScope::addChild):
(WebCore::OverlapMapContainer::mergeClippingScopesRecursive):

• rendering/RenderLayerCompositor.cpp:

(WebCore::RenderLayerCompositor::flushPendingLayerChanges): Drive-by fix: m_overflowControlsHostLayer is null on iOS, so use rootGraphicsLayer().

Tools:

Add code to load a page by default in MobileMiniBrowser so it's easy to hack it
to load a test page of your choice.

• MobileMiniBrowser/MobileMiniBrowserFramework/WebViewController.m:

(-[WebViewController viewDidLoad]):

Location:

trunk

Files:

• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/LayerOverlapMap.cpp (modified) (2 diffs)
• Source/WebCore/rendering/RenderLayerCompositor.cpp (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/MobileMiniBrowser/MobileMiniBrowserFramework/WebViewController.m (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-07-06  Simon Fraser  <simon.fraser@apple.com>
+
+        Long hang when loading a cnn.com page on iOS
+        https://bugs.webkit.org/show_bug.cgi?id=199556
+
+        Reviewed by Zalan Bujtas.
+        
+        Loading https://edition.cnn.com/travel/article/brussels-airlines-flight-to-nowhere/index.html in the iOS 13 sim
+        results in a long hang under OverlapMapContainer::append(). We were creating pathological clipping scopes with
+        thousands of entries, because OverlapMapContainer::mergeClippingScopesRecursive() had a logic error where
+        it added 'sourceScope' to the child instead of 'sourceChildScope'. Add a new assertion to detect that case.
+        
+        I wasn't able to create a testcase that caused a hang, but a number of existing tests would have
+        hit the assertion.
+
+        * rendering/LayerOverlapMap.cpp:
+        (WebCore::OverlapMapContainer::ClippingScope::addChild):
+        (WebCore::OverlapMapContainer::mergeClippingScopesRecursive):
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::flushPendingLayerChanges): Drive-by fix: m_overflowControlsHostLayer is null on iOS, so use rootGraphicsLayer().
+
 2019-07-05  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebCore/rendering/LayerOverlapMap.cpp

@@ -115 +115 @@
         ClippingScope* addChild(const ClippingScope& child)
         {
+            ASSERT(&layer != &child.layer);
             children.append(child);
             return &children.last();
@@ -199 +200 @@
         } else {
             // New child, just copy the whole subtree.
-            destScope.addChild(sourceScope);
+            destScope.addChild(sourceChildScope);
         }
     }

trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp

@@ -524 +524 @@
         if (layersLogEnabled()) {
             LOG(Layers, "RenderLayerCompositor::flushPendingLayerChanges");
-            showGraphicsLayerTree(m_overflowControlsHostLayer.get());
+            showGraphicsLayerTree(rootGraphicsLayer());
         }
 #endif

trunk/Tools/ChangeLog

@@ +1 @@
+2019-07-06  Simon Fraser  <simon.fraser@apple.com>
+
+        Long hang when loading a cnn.com page on iOS
+        https://bugs.webkit.org/show_bug.cgi?id=199556
+
+        Reviewed by Zalan Bujtas.
+
+        Add code to load a page by default in MobileMiniBrowser so it's easy to hack it
+        to load a test page of your choice.
+
+        * MobileMiniBrowser/MobileMiniBrowserFramework/WebViewController.m:
+        (-[WebViewController viewDidLoad]):
+
 2019-07-05  Ryosuke Niwa  <rniwa@webkit.org>
 

trunk/Tools/MobileMiniBrowser/MobileMiniBrowserFramework/WebViewController.m

@@ -76 +76 @@
     self.tabViewController.modalPresentationStyle = UIModalPresentationPopover;
 
-    [self setCurrentWebView:[self createWebView]];
+    WKWebView *webView = [self createWebView];
+    [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"https://webkit.org"]]];
+    [self setCurrentWebView:webView];
 }