Changeset 74723 in webkit

Timestamp:

Dec 28, 2010 1:57:11 PM (13 years ago)

Author:

tonikitoo@webkit.org

Message:

2010-12-20 Antonio Gomes <​agomes@rim.com>

Reviewed by Daniel Bates.

Spatial Navigation: code clean up (Part VI)
​https://bugs.webkit.org/show_bug.cgi?id=50666

No new tests needed.

• page/FocusController.cpp: (WebCore::updatFocusCandidateIfNeeded): Assert renderer() and isElementNode() now that we are bailing out earlier in both the FocusCandidate constructor and FocusController::findFocusCandidateInContainer().
• page/SpatialNavigation.h: Swapped the parameters order in canScrollInDirection and virtualRectForAreaElementAndDirection functions. (WebCore::FocusController::findFocusCandidateInContainer): (WebCore::FocusController::advanceFocusDirectionallyInContainer): Adjusted call sites of canScrollInDirection(), and added an early return if !isElementNode(). (WebCore::FocusController::advanceFocusDirectionally): Adjusted call site of virtualRectForAreaElementAndDirection().
• page/SpatialNavigation.cpp: (WebCore::FocusCandidate::FocusCandidate): Assert if node is not an Element node. (WebCore::isScrollableNode): Renamed from isScrollableContainerNode. (WebCore::scrollInDirection): Adjusted call site after function name change; (WebCore::scrollableEnclosingBoxOrParentFrameForNodeInDi:rection): Assert if node is a Document node. (WebCore::canScrollInDirection): Signature changed. (WebCore::canBeScrolledIntoView): Ditto. (WebCore::virtualRectForAreaElementAndDirection): Ditto.

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• page/FocusController.cpp (modified) (4 diffs)
• page/SpatialNavigation.cpp (modified) (10 diffs)
• page/SpatialNavigation.h (modified) (2 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-12-20  Antonio Gomes  <agomes@rim.com>
+
+        Reviewed by Daniel Bates.
+
+        Spatial Navigation: code clean up (Part VI)
+        https://bugs.webkit.org/show_bug.cgi?id=50666
+
+        No new tests needed.
+
+        * page/FocusController.cpp:
+        (WebCore::updatFocusCandidateIfNeeded): Assert renderer() and
+        isElementNode() now that we are bailing out earlier in both the
+        FocusCandidate constructor and FocusController::findFocusCandidateInContainer().
+        * page/SpatialNavigation.h: Swapped the parameters order in canScrollInDirection
+        and virtualRectForAreaElementAndDirection functions.
+        (WebCore::FocusController::findFocusCandidateInContainer):
+        (WebCore::FocusController::advanceFocusDirectionallyInContainer): Adjusted callsites
+        of canScrollInDirection(), and added an early return if !isElementNode().
+        (WebCore::FocusController::advanceFocusDirectionally): Adjusted callsite of
+        virtualRectForAreaElementAndDirection();
+        * page/SpatialNavigation.cpp:
+        (WebCore::FocusCandidate::FocusCandidate): Assert if node is not a element node;
+        (WebCore::isScrollableNode): Renamed from isScrollableContainerNode;
+        (WebCore::scrollInDirection): Adjusted callsite after function name change;
+        (WebCore::scrollableEnclosingBoxOrParentFrameForNodeInDi:rection): Assert if node is
+        a documentNode.
+        (WebCore::canScrollInDirection): Signature changed.
+        (WebCore::canBeScrolledIntoView): Ditto.
+        (WebCore::virtualRectForAreaElementAndDirection): Ditto.
+
 2010-12-28  Adrienne Walker  <enne@google.com>
 

trunk/WebCore/page/FocusController.cpp

@@ -418 +418 @@
 static void updateFocusCandidateIfNeeded(FocusDirection direction, const IntRect& startingRect, FocusCandidate& candidate, FocusCandidate& closest)
 {
-    if (!candidate.visibleNode->isElementNode() || !candidate.visibleNode->renderer())
-        return;
+    ASSERT(candidate.visibleNode->isElementNode());
+    ASSERT(candidate.visibleNode->renderer());
 
     // Ignore iframes that don't have a src attribute
@@ -473 +473 @@
 
     Node* node = container->firstChild();
-    for (; node; node = (node->isFrameOwnerElement() || canScrollInDirection(direction, node)) ? node->traverseNextSibling(container) : node->traverseNextNode(container)) {
+    for (; node; node = (node->isFrameOwnerElement() || canScrollInDirection(node, direction)) ? node->traverseNextSibling(container) : node->traverseNextNode(container)) {
         if (node == focusedNode)
             continue;
 
-        if (!node->isKeyboardFocusable(event) && !node->isFrameOwnerElement() && !canScrollInDirection(direction, node))
+        if (!node->isElementNode())
+            continue;
+
+        if (!node->isKeyboardFocusable(event) && !node->isFrameOwnerElement() && !canScrollInDirection(node, direction))
             continue;
 
@@ -532 +535 @@
         return true;
     }
-    if (canScrollInDirection(direction, focusCandidate.visibleNode)) {
+
+    if (canScrollInDirection(focusCandidate.visibleNode, direction)) {
         if (focusCandidate.isOffscreenAfterScrolling) {
             scrollInDirection(focusCandidate.visibleNode, direction);
@@ -579 +583 @@
             HTMLAreaElement* area = static_cast<HTMLAreaElement*>(focusedNode);
             container = scrollableEnclosingBoxOrParentFrameForNodeInDirection(direction, area->imageElement());
-            startingRect = virtualRectForAreaElementAndDirection(direction, area);
+            startingRect = virtualRectForAreaElementAndDirection(area, direction);
         }
     }

trunk/WebCore/page/SpatialNavigation.cpp

@@ -53 +53 @@
 static IntRect rectToAbsoluteCoordinates(Frame* initialFrame, const IntRect&);
 static void entryAndExitPointsForDirection(FocusDirection direction, const IntRect& startingRect, const IntRect& potentialRect, IntPoint& exitPoint, IntPoint& entryPoint);
-static bool isScrollableContainerNode(const Node*);
+static bool isScrollableNode(const Node*);
 
 FocusCandidate::FocusCandidate(Node* node, FocusDirection direction)
@@ -67 +67 @@
 {
     ASSERT(node);
+    ASSERT(node->isElementNode());
+
     if (node->hasTagName(HTMLNames::areaTag)) {
         HTMLAreaElement* area = static_cast<HTMLAreaElement*>(node);
@@ -74 +76 @@
 
         visibleNode = image;
-        rect = virtualRectForAreaElementAndDirection(direction, area);
+        rect = virtualRectForAreaElementAndDirection(area, direction);
     } else {
         if (!node->renderer())
@@ -337 +339 @@
     ASSERT(frame);
 
-    if (frame && canScrollInDirection(direction, frame->document())) {
+    if (frame && canScrollInDirection(frame->document(), direction)) {
         int dx = 0;
         int dy = 0;
@@ -373 +375 @@
         return false;
 
-    if (canScrollInDirection(direction, container)) {
+    if (canScrollInDirection(container, direction)) {
         int dx = 0;
         int dy = 0;
@@ -418 +420 @@
 }
 
-bool isScrollableContainerNode(const Node* node)
-{
+bool isScrollableNode(const Node* node)
+{
+    ASSERT(!node->isDocumentNode());
+
     if (!node)
         return false;
 
-    if (RenderObject* renderer = node->renderer()) {
-        return (renderer->isBox() && toRenderBox(renderer)->canBeScrolledAndHasScrollableArea()
-             && node->hasChildNodes() && !node->isDocumentNode());
-    }
+    if (RenderObject* renderer = node->renderer())
+        return renderer->isBox() && toRenderBox(renderer)->canBeScrolledAndHasScrollableArea() && node->hasChildNodes();
 
     return false;
@@ -440 +442 @@
         else
             parent = parent->parentNode();
-    } while (parent && !canScrollInDirection(direction, parent) && !parent->isDocumentNode());
+    } while (parent && !canScrollInDirection(parent, direction) && !parent->isDocumentNode());
 
     return parent;
 }
 
-bool canScrollInDirection(FocusDirection direction, const Node* container)
+bool canScrollInDirection(const Node* container, FocusDirection direction)
 {
     ASSERT(container);
     if (container->isDocumentNode())
-        return canScrollInDirection(direction, static_cast<const Document*>(container)->frame());
-
-    if (!isScrollableContainerNode(container))
+        return canScrollInDirection(static_cast<const Document*>(container)->frame(), direction);
+
+    if (!isScrollableNode(container))
         return false;
 
@@ -469 +471 @@
 }
 
-bool canScrollInDirection(FocusDirection direction, const Frame* frame)
+bool canScrollInDirection(const Frame* frame, FocusDirection direction)
 {
     if (!frame->view())
@@ -660 +662 @@
         }
         if (parentNode == candidate.enclosingScrollableBox)
-            return canScrollInDirection(direction, parentNode);
+            return canScrollInDirection(parentNode, direction);
     }
     return true;
@@ -694 +696 @@
 }
 
-IntRect virtualRectForAreaElementAndDirection(FocusDirection direction, HTMLAreaElement* area)
+IntRect virtualRectForAreaElementAndDirection(HTMLAreaElement* area, FocusDirection direction)
 {
     ASSERT(area);

trunk/WebCore/page/SpatialNavigation.h

@@ -139 +139 @@
 bool scrollInDirection(Frame*, FocusDirection);
 bool scrollInDirection(Node* container, FocusDirection);
-bool canScrollInDirection(FocusDirection, const Node* container);
-bool canScrollInDirection(FocusDirection, const Frame*);
+bool canScrollInDirection(const Node* container, FocusDirection);
+bool canScrollInDirection(const Frame*, FocusDirection);
 bool canBeScrolledIntoView(FocusDirection, const FocusCandidate&);
 void distanceDataForNode(FocusDirection, const FocusCandidate& current, FocusCandidate& candidate);
@@ -147 +147 @@
 IntRect frameRectInAbsoluteCoordinates(Frame*);
 IntRect virtualRectForDirection(FocusDirection, const IntRect& startingRect, int width = 0);
-IntRect virtualRectForAreaElementAndDirection(FocusDirection, HTMLAreaElement*);
+IntRect virtualRectForAreaElementAndDirection(HTMLAreaElement*, FocusDirection);
 HTMLFrameOwnerElement* frameOwnerElement(FocusCandidate&);