Changeset 205290 in webkit

Timestamp:

Sep 1, 2016 7:22:33 AM (8 years ago)

Author:

akling@apple.com

Message:

FocusController should pass KeyboardEvent around by reference.
<​https://webkit.org/b/161461>

Reviewed by Sam Weinig.

Clean up FocusController to pass KeyboardEvent& around internally.

Also make FocusController::setInitialFocus() synthesize a dummy KeyboardEvent
if one isn't provided, just like nextFocusableElement()/previousFocusableElement() does.
This way we can feel confident about dereferencing the formerly KeyboardEvent* everywhere.

• page/EventHandler.cpp:

(WebCore::EventHandler::defaultArrowEventHandler):
(WebCore::EventHandler::defaultTabEventHandler):

• page/FocusController.cpp:

(WebCore::isFocusableElementOrScopeOwner):
(WebCore::isNonFocusableScopeOwner):
(WebCore::isFocusableScopeOwner):
(WebCore::shadowAdjustedTabIndex):
(WebCore::FocusController::findFocusableElementDescendingDownIntoFrameDocument):
(WebCore::FocusController::setInitialFocus):
(WebCore::FocusController::advanceFocus):
(WebCore::FocusController::advanceFocusInDocumentOrder):
(WebCore::FocusController::findFocusableElementAcrossFocusScope):
(WebCore::FocusController::findFocusableElementWithinScope):
(WebCore::FocusController::nextFocusableElementWithinScope):
(WebCore::FocusController::previousFocusableElementWithinScope):
(WebCore::FocusController::findFocusableElementOrScopeOwner):
(WebCore::FocusController::findElementWithExactTabIndex):
(WebCore::nextElementWithGreaterTabIndex):
(WebCore::previousElementWithLowerTabIndex):
(WebCore::FocusController::nextFocusableElement):
(WebCore::FocusController::previousFocusableElement):
(WebCore::FocusController::nextFocusableElementOrScopeOwner):
(WebCore::FocusController::previousFocusableElementOrScopeOwner):
(WebCore::FocusController::findFocusCandidateInContainer):
(WebCore::FocusController::advanceFocusDirectionallyInContainer):
(WebCore::FocusController::advanceFocusDirectionally):

• page/FocusController.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• page/EventHandler.cpp (modified) (2 diffs)
• page/FocusController.cpp (modified) (21 diffs)
• page/FocusController.h (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-09-01  Andreas Kling  <akling@apple.com>
+
+        FocusController should pass KeyboardEvent around by reference.
+        <https://webkit.org/b/161461>
+
+        Reviewed by Sam Weinig.
+
+        Clean up FocusController to pass KeyboardEvent& around internally.
+
+        Also make FocusController::setInitialFocus() synthesize a dummy KeyboardEvent
+        if one isn't provided, just like nextFocusableElement()/previousFocusableElement() does.
+        This way we can feel confident about dereferencing the formerly KeyboardEvent* everywhere.
+
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::defaultArrowEventHandler):
+        (WebCore::EventHandler::defaultTabEventHandler):
+        * page/FocusController.cpp:
+        (WebCore::isFocusableElementOrScopeOwner):
+        (WebCore::isNonFocusableScopeOwner):
+        (WebCore::isFocusableScopeOwner):
+        (WebCore::shadowAdjustedTabIndex):
+        (WebCore::FocusController::findFocusableElementDescendingDownIntoFrameDocument):
+        (WebCore::FocusController::setInitialFocus):
+        (WebCore::FocusController::advanceFocus):
+        (WebCore::FocusController::advanceFocusInDocumentOrder):
+        (WebCore::FocusController::findFocusableElementAcrossFocusScope):
+        (WebCore::FocusController::findFocusableElementWithinScope):
+        (WebCore::FocusController::nextFocusableElementWithinScope):
+        (WebCore::FocusController::previousFocusableElementWithinScope):
+        (WebCore::FocusController::findFocusableElementOrScopeOwner):
+        (WebCore::FocusController::findElementWithExactTabIndex):
+        (WebCore::nextElementWithGreaterTabIndex):
+        (WebCore::previousElementWithLowerTabIndex):
+        (WebCore::FocusController::nextFocusableElement):
+        (WebCore::FocusController::previousFocusableElement):
+        (WebCore::FocusController::nextFocusableElementOrScopeOwner):
+        (WebCore::FocusController::previousFocusableElementOrScopeOwner):
+        (WebCore::FocusController::findFocusCandidateInContainer):
+        (WebCore::FocusController::advanceFocusDirectionallyInContainer):
+        (WebCore::FocusController::advanceFocusDirectionally):
+        * page/FocusController.h:
+
 2016-09-01  Romain Bellessort  <romain.bellessort@crf.canon.fr>
 

trunk/Source/WebCore/page/EventHandler.cpp

@@ -3667 +3667 @@
         return;
 
-    if (page->focusController().advanceFocus(focusDirection, &event))
+    if (page->focusController().advanceFocus(focusDirection, event))
         event.setDefaultHandled();
 }
@@ -3691 +3691 @@
         return;
 
-    if (page->focusController().advanceFocus(focusDirection, &event))
+    if (page->focusController().advanceFocus(focusDirection, event))
         event.setDefaultHandled();
 }

trunk/Source/WebCore/page/FocusController.cpp

@@ -290 +290 @@
 }
 
-static inline bool isFocusableElementOrScopeOwner(Element& element, KeyboardEvent* event)
-{
-    return element.isKeyboardFocusable(*event) || isFocusScopeOwner(element);
-}
-
-static inline bool isNonFocusableScopeOwner(Element& element, KeyboardEvent* event)
-{
-    return !element.isKeyboardFocusable(*event) && isFocusScopeOwner(element);
-}
-
-static inline bool isFocusableScopeOwner(Element& element, KeyboardEvent* event)
-{
-    return element.isKeyboardFocusable(*event) && isFocusScopeOwner(element);
-}
-
-static inline int shadowAdjustedTabIndex(Element& element, KeyboardEvent* event)
+static inline bool isFocusableElementOrScopeOwner(Element& element, KeyboardEvent& event)
+{
+    return element.isKeyboardFocusable(event) || isFocusScopeOwner(element);
+}
+
+static inline bool isNonFocusableScopeOwner(Element& element, KeyboardEvent& event)
+{
+    return !element.isKeyboardFocusable(event) && isFocusScopeOwner(element);
+}
+
+static inline bool isFocusableScopeOwner(Element& element, KeyboardEvent& event)
+{
+    return element.isKeyboardFocusable(event) && isFocusScopeOwner(element);
+}
+
+static inline int shadowAdjustedTabIndex(Element& element, KeyboardEvent& event)
 {
     if (isNonFocusableScopeOwner(element, event)) {
@@ -377 +377 @@
 }
 
-Element* FocusController::findFocusableElementDescendingDownIntoFrameDocument(FocusDirection direction, Element* element, KeyboardEvent* event)
+Element* FocusController::findFocusableElementDescendingDownIntoFrameDocument(FocusDirection direction, Element* element, KeyboardEvent& event)
 {
     // The node we found might be a HTMLFrameOwnerElement, so descend down the tree until we find either:
@@ -395 +395 @@
 }
 
-bool FocusController::setInitialFocus(FocusDirection direction, KeyboardEvent* event)
-{
-    bool didAdvanceFocus = advanceFocus(direction, event, true);
+bool FocusController::setInitialFocus(FocusDirection direction, KeyboardEvent* providedEvent)
+{
+    RefPtr<KeyboardEvent> event = providedEvent;
+    if (!event)
+        event = KeyboardEvent::createForDummy();
+
+    bool didAdvanceFocus = advanceFocus(direction, *event, true);
     
     // If focus is being set initially, accessibility needs to be informed that system focus has moved 
@@ -408 +412 @@
 }
 
-bool FocusController::advanceFocus(FocusDirection direction, KeyboardEvent* event, bool initialFocus)
+bool FocusController::advanceFocus(FocusDirection direction, KeyboardEvent& event, bool initialFocus)
 {
     switch (direction) {
@@ -426 +430 @@
 }
 
-bool FocusController::advanceFocusInDocumentOrder(FocusDirection direction, KeyboardEvent* event, bool initialFocus)
+bool FocusController::advanceFocusInDocumentOrder(FocusDirection direction, KeyboardEvent& event, bool initialFocus)
 {
     Frame& frame = focusedOrMainFrame();
@@ -465 +469 @@
     }
 
-    if (is<HTMLFrameOwnerElement>(*element) && (!is<HTMLPlugInElement>(*element) || !element->isKeyboardFocusable(*event))) {
+    if (is<HTMLFrameOwnerElement>(*element) && (!is<HTMLPlugInElement>(*element) || !element->isKeyboardFocusable(event))) {
         // We focus frames rather than frame owners.
         // FIXME: We should not focus frames that have no scrollbars, as focusing them isn't useful to the user.
@@ -503 +507 @@
 }
 
-Element* FocusController::findFocusableElementAcrossFocusScope(FocusDirection direction, const FocusNavigationScope& scope, Node* currentNode, KeyboardEvent* event)
+Element* FocusController::findFocusableElementAcrossFocusScope(FocusDirection direction, const FocusNavigationScope& scope, Node* currentNode, KeyboardEvent& event)
 {
     ASSERT(!is<Element>(currentNode) || !isNonFocusableScopeOwner(downcast<Element>(*currentNode), event));
@@ -529 +533 @@
 }
 
-Element* FocusController::findFocusableElementWithinScope(FocusDirection direction, const FocusNavigationScope& scope, Node* start, KeyboardEvent* event)
+Element* FocusController::findFocusableElementWithinScope(FocusDirection direction, const FocusNavigationScope& scope, Node* start, KeyboardEvent& event)
 {
     // Starting node is exclusive.
@@ -538 +542 @@
 }
 
-Element* FocusController::nextFocusableElementWithinScope(const FocusNavigationScope& scope, Node* start, KeyboardEvent* event)
+Element* FocusController::nextFocusableElementWithinScope(const FocusNavigationScope& scope, Node* start, KeyboardEvent& event)
 {
     Element* found = nextFocusableElementOrScopeOwner(scope, start, event);
@@ -551 +555 @@
 }
 
-Element* FocusController::previousFocusableElementWithinScope(const FocusNavigationScope& scope, Node* start, KeyboardEvent* event)
+Element* FocusController::previousFocusableElementWithinScope(const FocusNavigationScope& scope, Node* start, KeyboardEvent& event)
 {
     Element* found = previousFocusableElementOrScopeOwner(scope, start, event);
@@ -570 +574 @@
 }
 
-Element* FocusController::findFocusableElementOrScopeOwner(FocusDirection direction, const FocusNavigationScope& scope, Node* node, KeyboardEvent* event)
+Element* FocusController::findFocusableElementOrScopeOwner(FocusDirection direction, const FocusNavigationScope& scope, Node* node, KeyboardEvent& event)
 {
     return (direction == FocusDirectionForward)
@@ -577 +581 @@
 }
 
-Element* FocusController::findElementWithExactTabIndex(const FocusNavigationScope& scope, Node* start, int tabIndex, KeyboardEvent* event, FocusDirection direction)
+Element* FocusController::findElementWithExactTabIndex(const FocusNavigationScope& scope, Node* start, int tabIndex, KeyboardEvent& event, FocusDirection direction)
 {
     // Search is inclusive of start
@@ -590 +594 @@
 }
 
-static Element* nextElementWithGreaterTabIndex(const FocusNavigationScope& scope, int tabIndex, KeyboardEvent* event)
+static Element* nextElementWithGreaterTabIndex(const FocusNavigationScope& scope, int tabIndex, KeyboardEvent& event)
 {
     // Search is inclusive of start
@@ -609 +613 @@
 }
 
-static Element* previousElementWithLowerTabIndex(const FocusNavigationScope& scope, Node* start, int tabIndex, KeyboardEvent* event)
+static Element* previousElementWithLowerTabIndex(const FocusNavigationScope& scope, Node* start, int tabIndex, KeyboardEvent& event)
 {
     // Search is inclusive of start
@@ -631 +635 @@
     // FIXME: This can return a non-focusable shadow host.
     Ref<KeyboardEvent> keyEvent = KeyboardEvent::createForDummy();
-    return nextFocusableElementOrScopeOwner(FocusNavigationScope::scopeOf(start), &start, keyEvent.ptr());
+    return nextFocusableElementOrScopeOwner(FocusNavigationScope::scopeOf(start), &start, keyEvent.get());
 }
 
@@ -638 +642 @@
     // FIXME: This can return a non-focusable shadow host.
     Ref<KeyboardEvent> keyEvent = KeyboardEvent::createForDummy();
-    return previousFocusableElementOrScopeOwner(FocusNavigationScope::scopeOf(start), &start, keyEvent.ptr());
-}
-
-Element* FocusController::nextFocusableElementOrScopeOwner(const FocusNavigationScope& scope, Node* start, KeyboardEvent* event)
+    return previousFocusableElementOrScopeOwner(FocusNavigationScope::scopeOf(start), &start, keyEvent.get());
+}
+
+Element* FocusController::nextFocusableElementOrScopeOwner(const FocusNavigationScope& scope, Node* start, KeyboardEvent& event)
 {
     int startTabIndex = 0;
@@ -678 +682 @@
 }
 
-Element* FocusController::previousFocusableElementOrScopeOwner(const FocusNavigationScope& scope, Node* start, KeyboardEvent* event)
+Element* FocusController::previousFocusableElementOrScopeOwner(const FocusNavigationScope& scope, Node* start, KeyboardEvent& event)
 {
     Node* last = nullptr;
@@ -938 +942 @@
 }
 
-void FocusController::findFocusCandidateInContainer(Node& container, const LayoutRect& startingRect, FocusDirection direction, KeyboardEvent* event, FocusCandidate& closest)
+void FocusController::findFocusCandidateInContainer(Node& container, const LayoutRect& startingRect, FocusDirection direction, KeyboardEvent& event, FocusCandidate& closest)
 {
     Node* focusedNode = (focusedFrame() && focusedFrame()->document()) ? focusedFrame()->document()->focusedElement() : 0;
@@ -955 +959 @@
             continue;
 
-        if (!element->isKeyboardFocusable(*event) && !element->isFrameOwnerElement() && !canScrollInDirection(element, direction))
+        if (!element->isKeyboardFocusable(event) && !element->isFrameOwnerElement() && !canScrollInDirection(element, direction))
             continue;
 
@@ -978 +982 @@
 }
 
-bool FocusController::advanceFocusDirectionallyInContainer(Node* container, const LayoutRect& startingRect, FocusDirection direction, KeyboardEvent* event)
+bool FocusController::advanceFocusDirectionallyInContainer(Node* container, const LayoutRect& startingRect, FocusDirection direction, KeyboardEvent& event)
 {
     if (!container)
@@ -1048 +1052 @@
 }
 
-bool FocusController::advanceFocusDirectionally(FocusDirection direction, KeyboardEvent* event)
+bool FocusController::advanceFocusDirectionally(FocusDirection direction, KeyboardEvent& event)
 {
     Document* focusedDocument = focusedOrMainFrame().document();

trunk/Source/WebCore/page/FocusController.h

@@ -60 +60 @@
 
     WEBCORE_EXPORT bool setInitialFocus(FocusDirection, KeyboardEvent*);
-    bool advanceFocus(FocusDirection, KeyboardEvent*, bool initialFocus = false);
+    bool advanceFocus(FocusDirection, KeyboardEvent&, bool initialFocus = false);
 
     WEBCORE_EXPORT bool setFocusedElement(Element*, PassRefPtr<Frame>, FocusDirection = FocusDirectionNone);
@@ -86 +86 @@
     void setIsVisibleAndActiveInternal(bool);
 
-    bool advanceFocusDirectionally(FocusDirection, KeyboardEvent*);
-    bool advanceFocusInDocumentOrder(FocusDirection, KeyboardEvent*, bool initialFocus);
+    bool advanceFocusDirectionally(FocusDirection, KeyboardEvent&);
+    bool advanceFocusInDocumentOrder(FocusDirection, KeyboardEvent&, bool initialFocus);
 
-    Element* findFocusableElementAcrossFocusScope(FocusDirection, const FocusNavigationScope& startScope, Node* start, KeyboardEvent*);
+    Element* findFocusableElementAcrossFocusScope(FocusDirection, const FocusNavigationScope& startScope, Node* start, KeyboardEvent&);
 
-    Element* findFocusableElementWithinScope(FocusDirection, const FocusNavigationScope&, Node* start, KeyboardEvent*);
-    Element* nextFocusableElementWithinScope(const FocusNavigationScope&, Node* start, KeyboardEvent*);
-    Element* previousFocusableElementWithinScope(const FocusNavigationScope&, Node* start, KeyboardEvent*);
+    Element* findFocusableElementWithinScope(FocusDirection, const FocusNavigationScope&, Node* start, KeyboardEvent&);
+    Element* nextFocusableElementWithinScope(const FocusNavigationScope&, Node* start, KeyboardEvent&);
+    Element* previousFocusableElementWithinScope(const FocusNavigationScope&, Node* start, KeyboardEvent&);
 
-    Element* findFocusableElementDescendingDownIntoFrameDocument(FocusDirection, Element*, KeyboardEvent*);
+    Element* findFocusableElementDescendingDownIntoFrameDocument(FocusDirection, Element*, KeyboardEvent&);
 
     // Searches through the given tree scope, starting from start node, for the next/previous selectable element that comes after/before start node.
@@ -106 +106 @@
     //
     // See http://www.w3.org/TR/html4/interact/forms.html#h-17.11.1
-    Element* findFocusableElementOrScopeOwner(FocusDirection, const FocusNavigationScope&, Node* start, KeyboardEvent*);
+    Element* findFocusableElementOrScopeOwner(FocusDirection, const FocusNavigationScope&, Node* start, KeyboardEvent&);
 
-    Element* findElementWithExactTabIndex(const FocusNavigationScope&, Node* start, int tabIndex, KeyboardEvent*, FocusDirection);
+    Element* findElementWithExactTabIndex(const FocusNavigationScope&, Node* start, int tabIndex, KeyboardEvent&, FocusDirection);
     
-    Element* nextFocusableElementOrScopeOwner(const FocusNavigationScope&, Node* start, KeyboardEvent*);
-    Element* previousFocusableElementOrScopeOwner(const FocusNavigationScope&, Node* start, KeyboardEvent*);
+    Element* nextFocusableElementOrScopeOwner(const FocusNavigationScope&, Node* start, KeyboardEvent&);
+    Element* previousFocusableElementOrScopeOwner(const FocusNavigationScope&, Node* start, KeyboardEvent&);
 
-    bool advanceFocusDirectionallyInContainer(Node* container, const LayoutRect& startingRect, FocusDirection, KeyboardEvent*);
-    void findFocusCandidateInContainer(Node& container, const LayoutRect& startingRect, FocusDirection, KeyboardEvent*, FocusCandidate& closest);
+    bool advanceFocusDirectionallyInContainer(Node* container, const LayoutRect& startingRect, FocusDirection, KeyboardEvent&);
+    void findFocusCandidateInContainer(Node& container, const LayoutRect& startingRect, FocusDirection, KeyboardEvent&, FocusCandidate& closest);
 
     void focusRepaintTimerFired();