Long hang when loading a cnn.com page on iOS
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 7 Jul 2019 00:20:03 +0000 (00:20 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 7 Jul 2019 00:20:03 +0000 (00:20 +0000)
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]):

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/LayerOverlapMap.cpp
Source/WebCore/rendering/RenderLayerCompositor.cpp
Tools/ChangeLog
Tools/MobileMiniBrowser/MobileMiniBrowserFramework/WebViewController.m

index 3ad5fed..313c764 100644 (file)
@@ -1,3 +1,24 @@
+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>
 
         Carvana.com needs the fetch AbortSignal quirk
index 794a483..285aa5f 100644 (file)
@@ -114,6 +114,7 @@ private:
 
         ClippingScope* addChild(const ClippingScope& child)
         {
+            ASSERT(&layer != &child.layer);
             children.append(child);
             return &children.last();
         }
@@ -198,7 +199,7 @@ void OverlapMapContainer::mergeClippingScopesRecursive(const ClippingScope& sour
             mergeClippingScopesRecursive(sourceChildScope, *destChild);
         } else {
             // New child, just copy the whole subtree.
-            destScope.addChild(sourceScope);
+            destScope.addChild(sourceChildScope);
         }
     }
 }
index e9b5d94..b665968 100644 (file)
@@ -523,7 +523,7 @@ void RenderLayerCompositor::flushPendingLayerChanges(bool isFlushRoot)
 #if ENABLE(TREE_DEBUGGING)
         if (layersLogEnabled()) {
             LOG(Layers, "RenderLayerCompositor::flushPendingLayerChanges");
-            showGraphicsLayerTree(m_overflowControlsHostLayer.get());
+            showGraphicsLayerTree(rootGraphicsLayer());
         }
 #endif
     }
index 7e06dc2..822e3ef 100644 (file)
@@ -1,3 +1,16 @@
+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>
 
         [iOS] Crash in WebKit::WebPage::positionInformation via Range::startPosition
index ecb227f..f1b5ede 100644 (file)
@@ -75,7 +75,9 @@ void* URLContext = &URLContext;
     self.tabViewController.parent = self;
     self.tabViewController.modalPresentationStyle = UIModalPresentationPopover;
 
-    [self setCurrentWebView:[self createWebView]];
+    WKWebView *webView = [self createWebView];
+    [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"https://webkit.org"]]];
+    [self setCurrentWebView:webView];
 }