[webkit-changes] [151581] trunk/Source/WebCore

Thu, 13 Jun 2013 21:57:01 -0700

Title: [151581] trunk/Source/WebCore
Revision
151581
Author
[email protected]
Date
2013-06-13 21:58:11 -0700 (Thu, 13 Jun 2013)

Log Message

Sometimes we stick in slow scrolling mode even after leaving a page
https://bugs.webkit.org/show_bug.cgi?id=117622

Reviewed by Sam Weinig.

ScrollingCoordinator, and thus the scrolling state tree, is owned by Page,
and so persists when navigating between cached pages. We do give the ScrollingStateTree
a new ScrollingStateScrollingNode on navigation, however the ScrollingStateScrollingNode
would not have dirty flags set for, say, WheelEventHandlerCount if the document had
no wheel handlers. And because that dirty flag wasn't set, ScrollingTree::commitNewTreeState()
would fail to update m_hasWheelEventHandlers, so we'd remain in slow scrolling.

Fix by having ScrollingStateTree remember if it's been given a new root ScrollingStateScrollingNode,
and making ScrollingTree::commitNewTreeState() to the right thing in that case.

Also fix a couple of issues with the tiled scrolling indicator. First, on cached page
navigation, the indicator color would show the state for the old page, because
ScrollingCoordinatorMac::commitTreeState() checked scrollingTree()->hasWheelEventHandlers()
before the scrolling thread has committed the new scrolling tree.

Second, the color change would animate, so stop that.

Not testable, since tests can only dump the scrolling state tree.

* page/scrolling/ScrollingStateTree.cpp:
(WebCore::ScrollingStateTree::ScrollingStateTree):
(WebCore::ScrollingStateTree::attachNode):
(WebCore::ScrollingStateTree::commit):
* page/scrolling/ScrollingStateTree.h:
(WebCore::ScrollingStateTree::hasNewRootStateNode):
* page/scrolling/ScrollingTree.cpp:
(WebCore::ScrollingTree::commitNewTreeState):
* page/scrolling/mac/ScrollingCoordinatorMac.mm:
(WebCore::ScrollingCoordinatorMac::commitTreeState):
* platform/graphics/ca/mac/TileController.mm:
(-[WebTiledScrollingIndicatorLayer init]): Turn off borderColor implicit animations.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (151580 => 151581)


--- trunk/Source/WebCore/ChangeLog	2013-06-14 04:47:13 UTC (rev 151580)
+++ trunk/Source/WebCore/ChangeLog	2013-06-14 04:58:11 UTC (rev 151581)
@@ -1,3 +1,42 @@
+2013-06-13  Simon Fraser  <[email protected]>
+
+        Sometimes we stick in slow scrolling mode even after leaving a page
+        https://bugs.webkit.org/show_bug.cgi?id=117622
+
+        Reviewed by Sam Weinig.
+        
+        ScrollingCoordinator, and thus the scrolling state tree, is owned by Page,
+        and so persists when navigating between cached pages. We do give the ScrollingStateTree
+        a new ScrollingStateScrollingNode on navigation, however the ScrollingStateScrollingNode
+        would not have dirty flags set for, say, WheelEventHandlerCount if the document had
+        no wheel handlers. And because that dirty flag wasn't set, ScrollingTree::commitNewTreeState()
+        would fail to update m_hasWheelEventHandlers, so we'd remain in slow scrolling.
+        
+        Fix by having ScrollingStateTree remember if it's been given a new root ScrollingStateScrollingNode,
+        and making ScrollingTree::commitNewTreeState() to the right thing in that case.
+        
+        Also fix a couple of issues with the tiled scrolling indicator. First, on cached page
+        navigation, the indicator color would show the state for the old page, because
+        ScrollingCoordinatorMac::commitTreeState() checked scrollingTree()->hasWheelEventHandlers()
+        before the scrolling thread has committed the new scrolling tree.
+        
+        Second, the color change would animate, so stop that.
+
+        Not testable, since tests can only dump the scrolling state tree.
+
+        * page/scrolling/ScrollingStateTree.cpp:
+        (WebCore::ScrollingStateTree::ScrollingStateTree):
+        (WebCore::ScrollingStateTree::attachNode):
+        (WebCore::ScrollingStateTree::commit):
+        * page/scrolling/ScrollingStateTree.h:
+        (WebCore::ScrollingStateTree::hasNewRootStateNode):
+        * page/scrolling/ScrollingTree.cpp:
+        (WebCore::ScrollingTree::commitNewTreeState):
+        * page/scrolling/mac/ScrollingCoordinatorMac.mm:
+        (WebCore::ScrollingCoordinatorMac::commitTreeState):
+        * platform/graphics/ca/mac/TileController.mm:
+        (-[WebTiledScrollingIndicatorLayer init]): Turn off borderColor implicit animations.
+
 2013-06-13  Peter Gal  <[email protected]>
 
         [curl] Merge http response header values

Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp (151580 => 151581)


--- trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp	2013-06-14 04:47:13 UTC (rev 151580)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateTree.cpp	2013-06-14 04:58:11 UTC (rev 151581)
@@ -41,6 +41,7 @@
 
 ScrollingStateTree::ScrollingStateTree()
     : m_hasChangedProperties(false)
+    , m_hasNewRootStateNode(false)
 {
 }
 
@@ -70,6 +71,7 @@
 
         setRootStateNode(ScrollingStateScrollingNode::create(this, newNodeID));
         newNode = rootStateNode();
+        m_hasNewRootStateNode = true;
     } else {
         ScrollingStateNode* parent = stateNodeForID(parentID);
         if (!parent)
@@ -136,6 +138,9 @@
     treeStateClone->m_hasChangedProperties = true;
     m_hasChangedProperties = false;
 
+    treeStateClone->m_hasNewRootStateNode = m_hasNewRootStateNode;
+    m_hasNewRootStateNode = false;
+
     return treeStateClone.release();
 }

Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h (151580 => 151581)


--- trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h	2013-06-14 04:47:13 UTC (rev 151580)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateTree.h	2013-06-14 04:58:11 UTC (rev 151581)
@@ -62,6 +62,8 @@
     void setHasChangedProperties(bool changedProperties) { m_hasChangedProperties = changedProperties; }
     bool hasChangedProperties() const { return m_hasChangedProperties; }
 
+    bool hasNewRootStateNode() const { return m_hasNewRootStateNode; }
+
 private:
     ScrollingStateTree();
 
@@ -75,6 +77,7 @@
     OwnPtr<ScrollingStateScrollingNode> m_rootStateNode;
     Vector<ScrollingNodeID> m_nodesRemovedSinceLastCommit;
     bool m_hasChangedProperties;
+    bool m_hasNewRootStateNode;
 };
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp (151580 => 151581)


--- trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp	2013-06-14 04:47:13 UTC (rev 151580)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp	2013-06-14 04:58:11 UTC (rev 151581)
@@ -132,18 +132,21 @@
 {
     ASSERT(ScrollingThread::isCurrentThread());
 
+    bool rootStateNodeChanged = scrollingStateTree->hasNewRootStateNode();
+    
     ScrollingStateScrollingNode* rootNode = scrollingStateTree->rootStateNode();
     if (rootNode
-        && (rootNode->hasChangedProperty(ScrollingStateScrollingNode::WheelEventHandlerCount)
+        && (rootStateNodeChanged
+            || rootNode->hasChangedProperty(ScrollingStateScrollingNode::WheelEventHandlerCount)
             || rootNode->hasChangedProperty(ScrollingStateScrollingNode::NonFastScrollableRegion)
             || rootNode->hasChangedProperty(ScrollingStateNode::ScrollLayer))) {
         MutexLocker lock(m_mutex);
 
-        if (rootNode->hasChangedProperty(ScrollingStateNode::ScrollLayer))
+        if (rootStateNodeChanged || rootNode->hasChangedProperty(ScrollingStateNode::ScrollLayer))
             m_mainFrameScrollPosition = IntPoint();
-        if (rootNode->hasChangedProperty(ScrollingStateScrollingNode::WheelEventHandlerCount))
+        if (rootStateNodeChanged || rootNode->hasChangedProperty(ScrollingStateScrollingNode::WheelEventHandlerCount))
             m_hasWheelEventHandlers = scrollingStateTree->rootStateNode()->wheelEventHandlerCount();
-        if (rootNode->hasChangedProperty(ScrollingStateScrollingNode::NonFastScrollableRegion))
+        if (rootStateNodeChanged || rootNode->hasChangedProperty(ScrollingStateScrollingNode::NonFastScrollableRegion))
             m_nonFastScrollableRegion = scrollingStateTree->rootStateNode()->nonFastScrollableRegion();
     }

Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm (151580 => 151581)


--- trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm	2013-06-14 04:47:13 UTC (rev 151580)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm	2013-06-14 04:58:11 UTC (rev 151581)
@@ -442,7 +442,7 @@
     ScrollingModeIndication indicatorMode;
     if (shouldUpdateScrollLayerPositionOnMainThread())
         indicatorMode = MainThreadScrollingBecauseOfStyleIndication;
-    else if (scrollingTree() && scrollingTree()->hasWheelEventHandlers())
+    else if (m_scrollingStateTree->rootStateNode()->wheelEventHandlerCount())
         indicatorMode =  MainThreadScrollingBecauseOfEventHandlersIndication;
     else
         indicatorMode = ThreadedScrollingIndication;

Modified: trunk/Source/WebCore/platform/graphics/ca/mac/TileController.mm (151580 => 151581)


--- trunk/Source/WebCore/platform/graphics/ca/mac/TileController.mm	2013-06-14 04:47:13 UTC (rev 151580)
+++ trunk/Source/WebCore/platform/graphics/ca/mac/TileController.mm	2013-06-14 04:58:11 UTC (rev 151581)
@@ -63,7 +63,7 @@
         [self setStyle:[NSDictionary dictionaryWithObject:[NSDictionary dictionaryWithObjectsAndKeys:[NSNull null], @"bounds", [NSNull null], @"position", [NSNull null], @"contents", nil] forKey:@"actions"]];
 
         _visibleRectFrameLayer = [CALayer layer];
-        [_visibleRectFrameLayer setStyle:[NSDictionary dictionaryWithObject:[NSDictionary dictionaryWithObjectsAndKeys:[NSNull null], @"bounds", [NSNull null], @"position", nil] forKey:@"actions"]];
+        [_visibleRectFrameLayer setStyle:[NSDictionary dictionaryWithObject:[NSDictionary dictionaryWithObjectsAndKeys:[NSNull null], @"bounds", [NSNull null], @"position", [NSNull null], @"borderColor", nil] forKey:@"actions"]];
         [self addSublayer:_visibleRectFrameLayer];
         [_visibleRectFrameLayer setBorderColor:WebCore::cachedCGColor(WebCore::Color(255, 0, 0), WebCore::ColorSpaceDeviceRGB)];
         [_visibleRectFrameLayer setBorderWidth:2];

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to