Addressing post-review comments in r177035

https://bugs.webkit.org/show_bug.cgi?id=139557

Patch by Myles C. Maxfield <mmaxfield@apple.com> on 2014-12-15
Reviewed by Darin Adler.

Source/WebCore:

This patch deletes the helper functions rendererBoundingBox() and rendererAnchorRect() and
migrates callers to using renderers directly.

It also improves the comment in RenderElement.h regarding RenderElement::anchorRect().

No new tests because this is simply refactoring.

* WebCore.exp.in: Delete exported symbol for rendererBoundingBox()
* accessibility/AccessibilitySlider.cpp:
(WebCore::AccessibilitySliderThumb::elementRect): Migrate off rendererBoundingBox()
* dom/ContainerNode.cpp:
(WebCore::rendererAnchorRect): Deleted.
* dom/ContainerNode.h:
* dom/Node.cpp:
(WebCore::rendererBoundingBox): Deleted.
* dom/Node.h:
* html/ColorInputType.cpp:
(WebCore::ColorInputType::elementRectRelativeToRootView): Migrate off rendererBoundingBox().
* html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::setupDateTimeChooserParameters): Ditto.
* html/ValidationMessage.cpp:
(WebCore::ValidationMessage::buildBubbleTree): Ditto.
* page/FrameView.cpp:
(WebCore::FrameView::scrollElementToRect): Migrate off rendererAnchorRect().
(WebCore::FrameView::scrollToAnchor): Ditto.
* page/SpatialNavigation.cpp:
(WebCore::nodeRectInAbsoluteCoordinates): Migrate off rendererBoundingBox().
* rendering/RenderElement.h:

Source/WebKit/mac:

* WebView/WebActionMenuController.mm:
(elementBoundingBoxInWindowCoordinatesFromNode): Migrate off rendererBoundingBox().

Source/WebKit2:

* Shared/WebHitTestResult.cpp:
(WebKit::WebHitTestResult::Data::elementBoundingBoxInWindowCoordinates): Migrate off rendererBoundingBox().
* WebProcess/WebPage/CoordinatedGraphics/WebPageCoordinatedGraphics.cpp:
(WebKit::WebPage::findZoomableAreaForPoint): Ditto.

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index ac3228cda30849ea433b8a8d3fd128902378a622..5297a750d4f9b931b872b7595163870b94f77d8c 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,39 @@
+2014-12-15  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Addressing post-review comments in r177035
+        https://bugs.webkit.org/show_bug.cgi?id=139557
+
+        Reviewed by Darin Adler.
+
+        This patch deletes the helper functions rendererBoundingBox() and rendererAnchorRect() and
+        migrates callers to using renderers directly.
+
+        It also improves the comment in RenderElement.h regarding RenderElement::anchorRect().
+
+        No new tests because this is simply refactoring.
+
+        * WebCore.exp.in: Delete exported symbol for rendererBoundingBox()
+        * accessibility/AccessibilitySlider.cpp:
+        (WebCore::AccessibilitySliderThumb::elementRect): Migrate off rendererBoundingBox()
+        * dom/ContainerNode.cpp:
+        (WebCore::rendererAnchorRect): Deleted.
+        * dom/ContainerNode.h:
+        * dom/Node.cpp:
+        (WebCore::rendererBoundingBox): Deleted.
+        * dom/Node.h:
+        * html/ColorInputType.cpp:
+        (WebCore::ColorInputType::elementRectRelativeToRootView): Migrate off rendererBoundingBox().
+        * html/HTMLInputElement.cpp:
+        (WebCore::HTMLInputElement::setupDateTimeChooserParameters): Ditto.
+        * html/ValidationMessage.cpp:
+        (WebCore::ValidationMessage::buildBubbleTree): Ditto.
+        * page/FrameView.cpp:
+        (WebCore::FrameView::scrollElementToRect): Migrate off rendererAnchorRect().
+        (WebCore::FrameView::scrollToAnchor): Ditto.
+        * page/SpatialNavigation.cpp:
+        (WebCore::nodeRectInAbsoluteCoordinates): Migrate off rendererBoundingBox().
+        * rendering/RenderElement.h:
+
 2014-12-15  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Delete Notation because we don't use it

diff --git a/Source/WebCore/WebCore.exp.in b/Source/WebCore/WebCore.exp.in
index 7a4b792ce2891d00b860dbe004418607a5ff2c09..b57bba3124a6da25093fc3892f4a33ec7c2b97c4 100644 (file)
--- a/Source/WebCore/WebCore.exp.in
+++ b/Source/WebCore/WebCore.exp.in
@@ -867,7 +867,6 @@ __ZN7WebCore19TextResourceDecoderD1Ev
 __ZN7WebCore19enclosingLayoutRectERKNS_9FloatRectE
 __ZN7WebCore19floatValueForLengthERKNS_6LengthEf
 __ZN7WebCore19getFileCreationTimeERKN3WTF6StringERl
-__ZN7WebCore19rendererBoundingBoxERKNS_4NodeE
 __ZN7WebCore19toInt32EnforceRangeEPN3JSC9ExecStateENS0_7JSValueE
 __ZN7WebCore20ApplicationCacheHost17maybeLoadResourceEPNS_14ResourceLoaderERKNS_15ResourceRequestERKNS_3URLE
 __ZN7WebCore20ApplicationCacheHost25maybeLoadFallbackForErrorEPNS_14ResourceLoaderERKNS_13ResourceErrorE

diff --git a/Source/WebCore/accessibility/AccessibilitySlider.cpp b/Source/WebCore/accessibility/AccessibilitySlider.cpp
index 4d235a9a052f3a2f7656adb620206321745d7673..15616509f19a91f54f88431bfdbc613186a006c3 100644 (file)
--- a/Source/WebCore/accessibility/AccessibilitySlider.cpp
+++ b/Source/WebCore/accessibility/AccessibilitySlider.cpp
@@ -159,7 +159,9 @@ LayoutRect AccessibilitySliderThumb::elementRect() const
     RenderObject* sliderRenderer = m_parent->renderer();
     if (!sliderRenderer || !sliderRenderer->isSlider())
         return LayoutRect();
-    return rendererBoundingBox(*downcast<HTMLInputElement>(sliderRenderer->node())->sliderThumbElement());
+    if (RenderElement* thumbRenderer = downcast<HTMLInputElement>(sliderRenderer->node())->sliderThumbElement()->renderer())
+        return thumbRenderer->absoluteBoundingBoxRect();
+    return LayoutRect();
 }
 
 bool AccessibilitySliderThumb::computeAccessibilityIsIgnored() const

diff --git a/Source/WebCore/dom/ContainerNode.cpp b/Source/WebCore/dom/ContainerNode.cpp
index ab4b1a3454e0bde00493826bcef53b6628e2f6ff..960960742b5f24dd2d79fda1396fbee592fe7144 100644 (file)
--- a/Source/WebCore/dom/ContainerNode.cpp
+++ b/Source/WebCore/dom/ContainerNode.cpp
@@ -910,11 +910,4 @@ RefPtr<RadioNodeList> ContainerNode::radioNodeList(const AtomicString& name)
     return ensureRareData().ensureNodeLists().addCacheWithAtomicName<RadioNodeList>(*this, name);
 }
 
-LayoutRect rendererAnchorRect(const ContainerNode& node)
-{
-    if (RenderElement* renderer = node.renderer())
-        return renderer->anchorRect();
-    return LayoutRect();
-}
-
 } // namespace WebCore

diff --git a/Source/WebCore/dom/ContainerNode.h b/Source/WebCore/dom/ContainerNode.h
index 944c33d805ff43b92a31659651060a84eb3d1773..f1855c8ac20a3cc279a1df8a30ea339eafb78ca9 100644 (file)
--- a/Source/WebCore/dom/ContainerNode.h
+++ b/Source/WebCore/dom/ContainerNode.h
@@ -304,8 +304,6 @@ private:
     ChildNodesLazySnapshot* m_nextSnapshot;
 };
 
-LayoutRect rendererAnchorRect(const ContainerNode&);
-
 } // namespace WebCore
 
 SPECIALIZE_TYPE_TRAITS_BEGIN(WebCore::ContainerNode)

diff --git a/Source/WebCore/dom/Node.cpp b/Source/WebCore/dom/Node.cpp
index 1cf00688bd466751c5e66bfd057456255730cd7e..299e693bfee186f3e6de4cf2eeb66ff8e23bcd73 100644 (file)
--- a/Source/WebCore/dom/Node.cpp
+++ b/Source/WebCore/dom/Node.cpp
@@ -2248,13 +2248,6 @@ bool Node::inRenderedDocument() const
     return inDocument() && document().hasLivingRenderTree();
 }
 
-IntRect rendererBoundingBox(const Node& node)
-{
-    if (RenderObject* renderer = node.renderer())
-        return renderer->absoluteBoundingBoxRect();
-    return IntRect();
-}
-
 } // namespace WebCore
 
 #ifndef NDEBUG

diff --git a/Source/WebCore/dom/Node.h b/Source/WebCore/dom/Node.h
index c6ef58fa396c0311c95e1947fac4706266823340..f2684753b8e6444b1ba67da1394f7ee229777e55 100644 (file)
--- a/Source/WebCore/dom/Node.h
+++ b/Source/WebCore/dom/Node.h
@@ -739,8 +739,6 @@ inline ContainerNode* Node::parentNodeGuaranteedHostFree() const
     return parentNode();
 }
 
-WEBCORE_EXPORT IntRect rendererBoundingBox(const Node&);
-
 } // namespace WebCore
 
 #ifndef NDEBUG

diff --git a/Source/WebCore/html/ColorInputType.cpp b/Source/WebCore/html/ColorInputType.cpp
index e6cf05208289cd062fd17808f7880e13cfad363f..496bc5712ecd8f2622c0cf85b3f780f62902939f 100644 (file)
--- a/Source/WebCore/html/ColorInputType.cpp
+++ b/Source/WebCore/html/ColorInputType.cpp
@@ -204,7 +204,9 @@ HTMLElement* ColorInputType::shadowColorSwatch() const
 
 IntRect ColorInputType::elementRectRelativeToRootView() const
 {
-    return element().document().view()->contentsToRootView(rendererBoundingBox(element()));
+    if (!element().renderer())
+        return IntRect();
+    element().document().view()->contentsToRootView(element().renderer()->absoluteBoundingBoxRect());
 }
 
 Color ColorInputType::currentColor()

diff --git a/Source/WebCore/html/HTMLInputElement.cpp b/Source/WebCore/html/HTMLInputElement.cpp
index 78924755492a902e20d1c76e83d9ba56894a247a..c580165f683c22a40d73150b44005f0810440c46 100644 (file)
--- a/Source/WebCore/html/HTMLInputElement.cpp
+++ b/Source/WebCore/html/HTMLInputElement.cpp
@@ -1877,7 +1877,10 @@ bool HTMLInputElement::setupDateTimeChooserParameters(DateTimeChooserParameters&
         parameters.stepBase = 0;
     }
 
-    parameters.anchorRectInRootView = document().view()->contentsToRootView(rendererBoundingBox(*this));
+    if (RenderObject* renderer = this->renderer())
+        parameters.anchorRectInRootView = document().view()->contentsToRootView(renderer->absoluteBoundingBoxRect());
+    else
+        parameters.anchorRectInRootView = IntRect();
     parameters.currentValue = value();
     parameters.isAnchorElementRTL = computedStyle()->direction() == RTL;
 #if ENABLE(DATALIST_ELEMENT)

diff --git a/Source/WebCore/html/ValidationMessage.cpp b/Source/WebCore/html/ValidationMessage.cpp
index cb4e7118978029c0eb84e4402c12f35b445c44c9..53b675bc2b494674e7bfc5ba39b69b66f895f0b8 100644 (file)
--- a/Source/WebCore/html/ValidationMessage.cpp
+++ b/Source/WebCore/html/ValidationMessage.cpp
@@ -168,6 +168,10 @@ static void adjustBubblePosition(const LayoutRect& hostRect, HTMLElement* bubble
 void ValidationMessage::buildBubbleTree()
 {
     ASSERT(!validationMessageClient());
+
+    if (!m_element->renderer())
+        return;
+
     ShadowRoot& shadowRoot = m_element->ensureUserAgentShadowRoot();
 
     Document& document = m_element->document();
@@ -178,7 +182,7 @@ void ValidationMessage::buildBubbleTree()
     m_bubble->setInlineStyleProperty(CSSPropertyPosition, CSSValueAbsolute);
     shadowRoot.appendChild(m_bubble.get(), ASSERT_NO_EXCEPTION);
     document.updateLayout();
-    adjustBubblePosition(rendererBoundingBox(*m_element), m_bubble.get());
+    adjustBubblePosition(m_element->renderer()->absoluteBoundingBoxRect(), m_bubble.get());
 
     RefPtr<HTMLDivElement> clipper = HTMLDivElement::create(document);
     clipper->setPseudo(AtomicString("-webkit-validation-bubble-arrow-clipper", AtomicString::ConstructFromLiteral));

diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
index 8117161279d95907758ace30dd1d8bcc2d9a4c9b..e8741ca32ee5ff6393f42ad5066a9cb7df85b755 100644 (file)
--- a/Source/WebCore/page/FrameView.cpp
+++ b/Source/WebCore/page/FrameView.cpp
@@ -2018,7 +2018,9 @@ void FrameView::scrollElementToRect(Element* element, const IntRect& rect)
 {
     frame().document()->updateLayoutIgnorePendingStylesheets();
 
-    LayoutRect bounds = rendererAnchorRect(*element);
+    LayoutRect bounds;
+    if (RenderElement* renderer = element->renderer())
+        bounds = renderer->anchorRect();
     int centeringOffsetX = (rect.width() - bounds.width()) / 2;
     int centeringOffsetY = (rect.height() - bounds.height()) / 2;
     setScrollPosition(IntPoint(bounds.x() - centeringOffsetX - rect.x(), bounds.y() - centeringOffsetY - rect.y()));
@@ -2787,8 +2789,8 @@ void FrameView::scrollToAnchor()
         return;
 
     LayoutRect rect;
-    if (anchorNode != frame().document())
-        rect = rendererAnchorRect(*anchorNode.get());
+    if (anchorNode != frame().document() && anchorNode->renderer())
+        rect = anchorNode->renderer()->anchorRect();
 
     // Scroll nested layers and frames to reveal the anchor.
     // Align to the top and to the closest side (this matches other browsers).

diff --git a/Source/WebCore/page/SpatialNavigation.cpp b/Source/WebCore/page/SpatialNavigation.cpp
index fd0f8fe6fa98ea5c67f58e4194bbbae99cf41fac..a4817b549d0f8a3a0470ae089d5b7d21d7d5972d 100644 (file)
--- a/Source/WebCore/page/SpatialNavigation.cpp
+++ b/Source/WebCore/page/SpatialNavigation.cpp
@@ -523,7 +523,9 @@ LayoutRect nodeRectInAbsoluteCoordinates(Node* node, bool ignoreBorder)
     if (is<Document>(*node))
         return frameRectInAbsoluteCoordinates(downcast<Document>(*node).frame());
 
-    LayoutRect rect = rectToAbsoluteCoordinates(node->document().frame(), rendererBoundingBox(*node));
+    LayoutRect rect;
+    if (RenderObject* renderer = node->renderer())
+        rect = rectToAbsoluteCoordinates(node->document().frame(), renderer->absoluteBoundingBoxRect());
 
     // For authors that use border instead of outline in their CSS, we compensate by ignoring the border when calculating
     // the rect of the focused element.

diff --git a/Source/WebCore/rendering/RenderElement.h b/Source/WebCore/rendering/RenderElement.h
index c30ccd0aa4541bb8ccf7fc2283c2ab1fd592501a..a25fba6a0fc158d71f296f0458396d2c7b57628d 100644 (file)
--- a/Source/WebCore/rendering/RenderElement.h
+++ b/Source/WebCore/rendering/RenderElement.h
@@ -151,7 +151,8 @@ public:
     bool hasHiddenBackface() const { return style().backfaceVisibility() == BackfaceVisibilityHidden; }
 
     // anchorRect() is conceptually similar to absoluteBoundingBoxRect(), but is intended for scrolling to an anchor.
-    // It works differently than absoluteBoundingBoxRect() for inline renderers.
+    // For inline renderers, this gets the logical top left of the first leaf child and the logical bottom right of the
+    // last leaf child, converts them to absolute coordinates, and makes a box out of them.
     LayoutRect anchorRect() const;
 
     bool hasFilter() const { return style().hasFilter(); }

diff --git a/Source/WebKit/mac/ChangeLog b/Source/WebKit/mac/ChangeLog
index ba663215807c58cf487ff83ea7e196d79702635f..f4925e5ae25e1f9d20bc579699daf53f6c24bcfb 100644 (file)
--- a/Source/WebKit/mac/ChangeLog
+++ b/Source/WebKit/mac/ChangeLog
@@ -1,3 +1,13 @@
+2014-12-15  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Addressing post-review comments in r177035
+        https://bugs.webkit.org/show_bug.cgi?id=139557
+
+        Reviewed by Darin Adler.
+
+        * WebView/WebActionMenuController.mm:
+        (elementBoundingBoxInWindowCoordinatesFromNode): Migrate off rendererBoundingBox().
+
 2014-12-15  Timothy Horton  <timothy_horton@apple.com>
 
         Implement Data Detectors immediate actions for Legacy WebKit

diff --git a/Source/WebKit/mac/WebView/WebActionMenuController.mm b/Source/WebKit/mac/WebView/WebActionMenuController.mm
index e0bd607197c78540212ecc0fabda18cb519775fd..bedf2d98d78256794aa412c44c419ec0034e350f 100644 (file)
--- a/Source/WebKit/mac/WebView/WebActionMenuController.mm
+++ b/Source/WebKit/mac/WebView/WebActionMenuController.mm
@@ -256,7 +256,11 @@ static IntRect elementBoundingBoxInWindowCoordinatesFromNode(Node* node)
     if (!view)
         return IntRect();
 
-    return view->contentsToWindow(rendererBoundingBox(*node));
+    RenderObject* renderer = node->renderer();
+    if (!renderer)
+        return IntRect();
+
+    return view->contentsToWindow(renderer->absoluteBoundingBoxRect());
 }
 
 - (NSArray *)_defaultMenuItemsForLink

diff --git a/Source/WebKit2/ChangeLog b/Source/WebKit2/ChangeLog
index 54eeb27ea270180a672df45717bf4df3a4bd5f07..230559defdaf0cd69847148bf382e984c17c669d 100644 (file)
--- a/Source/WebKit2/ChangeLog
+++ b/Source/WebKit2/ChangeLog
@@ -1,3 +1,15 @@
+2014-12-15  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Addressing post-review comments in r177035
+        https://bugs.webkit.org/show_bug.cgi?id=139557
+
+        Reviewed by Darin Adler.
+
+        * Shared/WebHitTestResult.cpp:
+        (WebKit::WebHitTestResult::Data::elementBoundingBoxInWindowCoordinates): Migrate off rendererBoundingBox().
+        * WebProcess/WebPage/CoordinatedGraphics/WebPageCoordinatedGraphics.cpp:
+        (WebKit::WebPage::findZoomableAreaForPoint): Ditto.
+
 2014-12-15  Timothy Horton  <timothy_horton@apple.com>
 
         Implement Data Detectors immediate actions for WebKit2

diff --git a/Source/WebKit2/Shared/WebHitTestResult.cpp b/Source/WebKit2/Shared/WebHitTestResult.cpp
index 4521f2d8bcf7f9b5931c2decb5321b4b9c75c65b..50d05ed50ca100db7941bf7d048db622450537bc 100644 (file)
--- a/Source/WebKit2/Shared/WebHitTestResult.cpp
+++ b/Source/WebKit2/Shared/WebHitTestResult.cpp
@@ -118,7 +118,11 @@ IntRect WebHitTestResult::Data::elementBoundingBoxInWindowCoordinates(const HitT
     if (!view)
         return IntRect();
 
-    return view->contentsToWindow(rendererBoundingBox(*node));
+    RenderObject* renderer = node->renderer();
+    if (!renderer)
+        return IntRect();
+
+    return view->contentsToWindow(renderer->absoluteBoundingBoxRect());
 }
 
 } // WebKit

diff --git a/Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/WebPageCoordinatedGraphics.cpp b/Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/WebPageCoordinatedGraphics.cpp
index 2bd37fe9a65f58b64306947830aef83b45b5d961..dd74aa08b405930f50fcd30ae12be7ad44b84baf 100644 (file)
--- a/Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/WebPageCoordinatedGraphics.cpp
+++ b/Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/WebPageCoordinatedGraphics.cpp
@@ -55,7 +55,9 @@ void WebPage::findZoomableAreaForPoint(const IntPoint& point, const IntSize& are
     if (!node)
         return;
 
-    IntRect zoomableArea = rendererBoundingBox(*node);
+    IntRect zoomableArea;
+    if (RenderObject* renderer = node->renderer())
+        zoomableArea = renderer->absoluteBoundingBoxRect();
 
     while (true) {
         bool found = !node->isTextNode() && !node->isShadowRoot();
@@ -71,7 +73,8 @@ void WebPage::findZoomableAreaForPoint(const IntPoint& point, const IntSize& are
             break;
 
         node = node->parentNode();
-        zoomableArea.unite(rendererBoundingBox(*node));
+        if (RenderObject* renderer = node.renderer())
+            zoomableArea.unite(renderer->absoluteBoundingBoxRect());
     }
 
     if (node->document().frame() && node->document().frame()->view()) {