Rename unscaledUnobscuredVisibleContentSize and unscaledVisibleContentSizeIncludingOb...
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 5 Nov 2016 00:55:43 +0000 (00:55 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 5 Nov 2016 00:55:43 +0000 (00:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=164438

Reviewed by Tim Horton.

unscaledUnobscuredVisibleContentSize() and unscaledVisibleContentSizeIncludingObscuredArea() were an endless source
of confusion.

Functions with "VisibleContent" in the name are usually expected to return document coordinates (affected by zooming),
so unscaledUnobscuredVisibleContentSize caused cognitive dissonance because of "unscaled" vs "visibleContent", and
"unobscured" vs "visible".

So rename:
    unscaledUnobscuredVisibleContentSize -> sizeForUnobscuredContent
    unscaledVisibleContentSizeIncludingObscuredArea -> sizeForVisibleContent

sizeForUnobscuredContent() can also be private to ScrollView.

* inspector/InspectorOverlay.cpp:
(WebCore::InspectorOverlay::update):
* platform/ScrollView.cpp:
(WebCore::ScrollView::unobscuredContentRectInternal):
(WebCore::ScrollView::sizeForVisibleContent):
(WebCore::ScrollView::sizeForUnobscuredContent): Don't compute unscaledVisibleContentSizeIncludingObscuredArea
before testing whether we have a platform widget.
(WebCore::ScrollView::layoutSize):
(WebCore::ScrollView::unscaledVisibleContentSizeIncludingObscuredArea): Deleted.
(WebCore::ScrollView::unscaledUnobscuredVisibleContentSize): Deleted.
* platform/ScrollView.h:
* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::calculateBackgroundImageGeometry):
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::flushPendingLayerChanges):
(WebCore::RenderLayerCompositor::frameViewDidChangeSize):
(WebCore::RenderLayerCompositor::updateRootLayerPosition):
(WebCore::RenderLayerCompositor::ensureRootLayer):

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

Source/WebCore/ChangeLog
Source/WebCore/inspector/InspectorOverlay.cpp
Source/WebCore/platform/ScrollView.cpp
Source/WebCore/platform/ScrollView.h
Source/WebCore/rendering/RenderBoxModelObject.cpp
Source/WebCore/rendering/RenderLayerCompositor.cpp

index b347bb0..ae64a49 100644 (file)
@@ -1,5 +1,44 @@
 2016-11-04  Simon Fraser  <simon.fraser@apple.com>
 
+        Rename unscaledUnobscuredVisibleContentSize and unscaledVisibleContentSizeIncludingObscuredArea for attempted clarity
+        https://bugs.webkit.org/show_bug.cgi?id=164438
+
+        Reviewed by Tim Horton.
+
+        unscaledUnobscuredVisibleContentSize() and unscaledVisibleContentSizeIncludingObscuredArea() were an endless source
+        of confusion.
+        
+        Functions with "VisibleContent" in the name are usually expected to return document coordinates (affected by zooming),
+        so unscaledUnobscuredVisibleContentSize caused cognitive dissonance because of "unscaled" vs "visibleContent", and
+        "unobscured" vs "visible".
+        
+        So rename:
+            unscaledUnobscuredVisibleContentSize -> sizeForUnobscuredContent
+            unscaledVisibleContentSizeIncludingObscuredArea -> sizeForVisibleContent
+        
+        sizeForUnobscuredContent() can also be private to ScrollView.
+
+        * inspector/InspectorOverlay.cpp:
+        (WebCore::InspectorOverlay::update):
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::unobscuredContentRectInternal):
+        (WebCore::ScrollView::sizeForVisibleContent):
+        (WebCore::ScrollView::sizeForUnobscuredContent): Don't compute unscaledVisibleContentSizeIncludingObscuredArea
+        before testing whether we have a platform widget.
+        (WebCore::ScrollView::layoutSize):
+        (WebCore::ScrollView::unscaledVisibleContentSizeIncludingObscuredArea): Deleted.
+        (WebCore::ScrollView::unscaledUnobscuredVisibleContentSize): Deleted.
+        * platform/ScrollView.h:
+        * rendering/RenderBoxModelObject.cpp:
+        (WebCore::RenderBoxModelObject::calculateBackgroundImageGeometry):
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::flushPendingLayerChanges):
+        (WebCore::RenderLayerCompositor::frameViewDidChangeSize):
+        (WebCore::RenderLayerCompositor::updateRootLayerPosition):
+        (WebCore::RenderLayerCompositor::ensureRootLayer):
+
+2016-11-04  Simon Fraser  <simon.fraser@apple.com>
+
         Layout viewport wrong with RTL documents
         https://bugs.webkit.org/show_bug.cgi?id=164434
 
index 951b5d5..5fca310 100644 (file)
@@ -322,8 +322,8 @@ void InspectorOverlay::update()
         return;
 
     FrameView* overlayView = overlayPage()->mainFrame().view();
-    IntSize viewportSize = view->unscaledVisibleContentSizeIncludingObscuredArea();
-    IntSize frameViewFullSize = view->unscaledVisibleContentSizeIncludingObscuredArea(ScrollableArea::IncludeScrollbars);
+    IntSize viewportSize = view->sizeForVisibleContent();
+    IntSize frameViewFullSize = view->sizeForVisibleContent(ScrollableArea::IncludeScrollbars);
     overlayView->resize(frameViewFullSize);
 
     // Clear canvas and paint things.
index 83104e3..c391deb 100644 (file)
@@ -247,12 +247,12 @@ IntRect ScrollView::unobscuredContentRect(VisibleContentRectIncludesScrollbars s
 
 IntRect ScrollView::unobscuredContentRectInternal(VisibleContentRectIncludesScrollbars scrollbarInclusion) const
 {
-    FloatSize visibleContentSize = unscaledUnobscuredVisibleContentSize(scrollbarInclusion);
+    FloatSize visibleContentSize = sizeForUnobscuredContent(scrollbarInclusion);
     visibleContentSize.scale(1 / visibleContentScaleFactor());
     return IntRect(m_scrollPosition, expandedIntSize(visibleContentSize));
 }
 
-IntSize ScrollView::unscaledVisibleContentSizeIncludingObscuredArea(VisibleContentRectIncludesScrollbars scrollbarInclusion) const
+IntSize ScrollView::sizeForVisibleContent(VisibleContentRectIncludesScrollbars scrollbarInclusion) const
 {
     if (platformWidget())
         return platformVisibleContentSizeIncludingObscuredArea(scrollbarInclusion == IncludeScrollbars);
@@ -269,13 +269,13 @@ IntSize ScrollView::unscaledVisibleContentSizeIncludingObscuredArea(VisibleConte
     return IntSize(width() - scrollbarSpace.width(), height() - scrollbarSpace.height()).expandedTo(IntSize());
 }
     
-IntSize ScrollView::unscaledUnobscuredVisibleContentSize(VisibleContentRectIncludesScrollbars scrollbarInclusion) const
+IntSize ScrollView::sizeForUnobscuredContent(VisibleContentRectIncludesScrollbars scrollbarInclusion) const
 {
-    IntSize visibleContentSize = unscaledVisibleContentSizeIncludingObscuredArea(scrollbarInclusion);
-
     if (platformWidget())
         return platformVisibleContentSize(scrollbarInclusion == IncludeScrollbars);
 
+    IntSize visibleContentSize = sizeForVisibleContent(scrollbarInclusion);
+
 #if USE(COORDINATED_GRAPHICS)
     if (m_useFixedLayout && !m_fixedVisibleContentRect.isEmpty())
         return visibleContentSize;
@@ -312,7 +312,7 @@ IntRect ScrollView::visibleContentRectInternal(VisibleContentRectIncludesScrollb
 
 IntSize ScrollView::layoutSize() const
 {
-    return m_fixedLayoutSize.isEmpty() || !m_useFixedLayout ? unscaledUnobscuredVisibleContentSize(ExcludeScrollbars) : m_fixedLayoutSize;
+    return m_fixedLayoutSize.isEmpty() || !m_useFixedLayout ? sizeForUnobscuredContent() : m_fixedLayoutSize;
 }
 
 IntSize ScrollView::fixedLayoutSize() const
index e26504f..e5c1821 100644 (file)
@@ -199,13 +199,9 @@ public:
 
     virtual bool inProgrammaticScroll() const { return false; }
 
-    // visibleContentRect().size() is computed from unscaledUnobscuredVisibleContentSize() divided by the value of visibleContentScaleFactor.
-    // visibleContentScaleFactor is usually 1, except when the setting delegatesPageScaling is true and the
-    // ScrollView is the main frame; in that case, visibleContentScaleFactor is equal to the page's pageScaleFactor.
-    // Ports that don't use pageScaleFactor can treat unscaledUnobscuredVisibleContentSize and visibleContentRect().size() as equivalent.
-    // unscaledVisibleContentSizeIncludingObscuredArea() includes areas in the content that might be obscured by UI elements.
-    IntSize unscaledUnobscuredVisibleContentSize(VisibleContentRectIncludesScrollbars = ExcludeScrollbars) const;
-    IntSize unscaledVisibleContentSizeIncludingObscuredArea(VisibleContentRectIncludesScrollbars = ExcludeScrollbars) const;
+    // Size available for view contents, including content inset areas. Not affected by zooming.
+    IntSize sizeForVisibleContent(VisibleContentRectIncludesScrollbars = ExcludeScrollbars) const;
+    // FIXME: remove this. It's only used for the incorrectly behaving ScrollView::unobscuredContentRectInternal().
     virtual float visibleContentScaleFactor() const { return 1; }
 
     // Functions for getting/setting the size webkit should use to layout the contents. By default this is the same as the visible
@@ -213,6 +209,7 @@ public:
     WEBCORE_EXPORT IntSize layoutSize() const;
     int layoutWidth() const { return layoutSize().width(); }
     int layoutHeight() const { return layoutSize().height(); }
+
     WEBCORE_EXPORT IntSize fixedLayoutSize() const;
     WEBCORE_EXPORT void setFixedLayoutSize(const IntSize&);
     WEBCORE_EXPORT bool useFixedLayout() const;
@@ -425,6 +422,9 @@ protected:
 #endif
 
 private:
+    // Size available for view contents, excluding content insets. Not affected by zooming.
+    IntSize sizeForUnobscuredContent(VisibleContentRectIncludesScrollbars = ExcludeScrollbars) const;
+
     IntRect visibleContentRectInternal(VisibleContentRectIncludesScrollbars, VisibleContentRectBehavior) const override;
     WEBCORE_EXPORT IntRect unobscuredContentRectInternal(VisibleContentRectIncludesScrollbars = ExcludeScrollbars) const;
 
index eb8d376..133bd79 100644 (file)
@@ -1120,7 +1120,7 @@ BackgroundImageGeometry RenderBoxModelObject::calculateBackgroundImageGeometry(c
                 // down the frameView to to fit in the current viewport.
                 viewportRect.setSize(frameView.fixedLayoutSize());
             } else
-                viewportRect.setSize(frameView.unscaledVisibleContentSizeIncludingObscuredArea());
+                viewportRect.setSize(frameView.sizeForVisibleContent());
 
             if (fixedBackgroundPaintsInLocalCoordinates()) {
                 if (!useFixedLayout) {
index 8b4fe33..3ca4049 100644 (file)
@@ -458,7 +458,7 @@ void RenderLayerCompositor::flushPendingLayerChanges(bool isFlushRoot)
         rootLayer->flushCompositingState(exposedRect, frameView.viewportIsStable());
 #else
         // Having a m_clipLayer indicates that we're doing scrolling via GraphicsLayers.
-        FloatRect visibleRect = m_clipLayer ? FloatRect({ 0, 0 }, frameView.unscaledVisibleContentSizeIncludingObscuredArea()) : frameView.visibleContentRect();
+        FloatRect visibleRect = m_clipLayer ? FloatRect({ 0, 0 }, frameView.sizeForVisibleContent()) : frameView.visibleContentRect();
 
         if (frameView.viewExposedRect())
             visibleRect.intersect(frameView.viewExposedRect().value());
@@ -1717,7 +1717,7 @@ void RenderLayerCompositor::frameViewDidChangeSize()
 {
     if (m_clipLayer) {
         const FrameView& frameView = m_renderView.frameView();
-        m_clipLayer->setSize(frameView.unscaledVisibleContentSizeIncludingObscuredArea());
+        m_clipLayer->setSize(frameView.sizeForVisibleContent());
         m_clipLayer->setPosition(positionForClipLayer());
 
         frameViewDidScroll();
@@ -2124,7 +2124,7 @@ void RenderLayerCompositor::updateRootLayerPosition()
         m_rootContentLayer->setAnchorPoint(FloatPoint3D());
     }
     if (m_clipLayer) {
-        m_clipLayer->setSize(m_renderView.frameView().unscaledVisibleContentSizeIncludingObscuredArea());
+        m_clipLayer->setSize(m_renderView.frameView().sizeForVisibleContent());
         m_clipLayer->setPosition(positionForClipLayer());
     }
 
@@ -3443,7 +3443,7 @@ void RenderLayerCompositor::ensureRootLayer()
             m_clipLayer->addChild(m_scrollLayer.get());
             m_scrollLayer->addChild(m_rootContentLayer.get());
 
-            m_clipLayer->setSize(m_renderView.frameView().unscaledVisibleContentSizeIncludingObscuredArea());
+            m_clipLayer->setSize(m_renderView.frameView().sizeForVisibleContent());
             m_clipLayer->setPosition(positionForClipLayer());
             m_clipLayer->setAnchorPoint(FloatPoint3D());