Changeset 204941 in webkit


Ignore:
Timestamp:
Aug 24, 2016 4:26:20 PM (8 years ago)
Author:
commit-queue@webkit.org
Message:

FocusController multiple dereferenced NULL pointers
​https://bugs.webkit.org/show_bug.cgi?id=160808

Patch by Jonathan Bedard <Jonathan Bedard> on 2016-08-24
Reviewed by Darin Adler.

Source/WebCore:

No new tests needed, fix does not change functionality.

This change fixes a number of NULL pointer dereferences which occur in FocusController.

  • page/FocusController.cpp:

(WebCore::isFocusableElementOrScopeOwner): Changed KeyboardEvent reference to pointer.
(WebCore::isNonFocusableScopeOwner): Ditto.
(WebCore::isFocusableScopeOwner): Ditto.
(WebCore::shadowAdjustedTabIndex): Ditto.

(WebCore::FocusController::findFocusableElementAcrossFocusScope): Pass pointer instead of reference to KeyboardEvent.
(WebCore::FocusController::nextFocusableElementWithinScope): Ditto.
(WebCore::FocusController::previousFocusableElementWithinScope): Ditto.
(WebCore::FocusController::findElementWithExactTabIndex): Ditto.
(WebCore::nextElementWithGreaterTabIndex): Ditto.
(WebCore::previousElementWithLowerTabIndex): Ditto.
(WebCore::FocusController::nextFocusableElementOrScopeOwner): Ditto.
(WebCore::FocusController::previousFocusableElementOrScopeOwner): Ditto.
(WebCore::relinquishesEditingFocus): Ditto.

Source/WebKit2:

  • WebProcess/WebPage/WebPage.cpp:

(WebKit::WebPage::setInitialFocus): Should use nullptr, not 0 to initialize NULL pointer.

Location:
trunk/Source
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r204938 r204941  
     12016-08-24  Jonathan Bedard  <jbedard@apple.com>
     2
     3        FocusController multiple dereferenced NULL pointers
     4        https://bugs.webkit.org/show_bug.cgi?id=160808
     5
     6        Reviewed by Darin Adler.
     7
     8        No new tests needed, fix does not change functionality.
     9
     10        This change fixes a number of NULL pointer dereferences which occur in FocusController.
     11
     12        * page/FocusController.cpp:
     13        (WebCore::isFocusableElementOrScopeOwner): Changed KeyboardEvent reference to pointer.
     14        (WebCore::isNonFocusableScopeOwner): Ditto.
     15        (WebCore::isFocusableScopeOwner): Ditto.
     16        (WebCore::shadowAdjustedTabIndex): Ditto.
     17
     18        (WebCore::FocusController::findFocusableElementAcrossFocusScope): Pass pointer instead of reference to KeyboardEvent.
     19        (WebCore::FocusController::nextFocusableElementWithinScope): Ditto.
     20        (WebCore::FocusController::previousFocusableElementWithinScope): Ditto.
     21        (WebCore::FocusController::findElementWithExactTabIndex): Ditto.
     22        (WebCore::nextElementWithGreaterTabIndex): Ditto.
     23        (WebCore::previousElementWithLowerTabIndex): Ditto.
     24        (WebCore::FocusController::nextFocusableElementOrScopeOwner): Ditto.
     25        (WebCore::FocusController::previousFocusableElementOrScopeOwner): Ditto.
     26        (WebCore::relinquishesEditingFocus): Ditto.
     27
    1282016-08-24  Nan Wang  <n_wang@apple.com>
    229

  • trunk/Source/WebCore/page/FocusController.cpp

    r202091 r204941  
    290290}
    291291
    292 static inline bool isFocusableElementOrScopeOwner(Element& element, KeyboardEvent& event)
    293 {
    294     return element.isKeyboardFocusable(&event) || isFocusScopeOwner(element);
    295 }
    296 
    297 static inline bool isNonFocusableScopeOwner(Element& element, KeyboardEvent& event)
    298 {
    299     return !element.isKeyboardFocusable(&event) && isFocusScopeOwner(element);
    300 }
    301 
    302 static inline bool isFocusableScopeOwner(Element& element, KeyboardEvent& event)
    303 {
    304     return element.isKeyboardFocusable(&event) && isFocusScopeOwner(element);
    305 }
    306 
    307 static inline int shadowAdjustedTabIndex(Element& element, KeyboardEvent& event)
     292static inline bool isFocusableElementOrScopeOwner(Element& element, KeyboardEvent* event)
     293{
     294    return element.isKeyboardFocusable(event) || isFocusScopeOwner(element);
     295}
     296
     297static inline bool isNonFocusableScopeOwner(Element& element, KeyboardEvent* event)
     298{
     299    return !element.isKeyboardFocusable(event) && isFocusScopeOwner(element);
     300}
     301
     302static inline bool isFocusableScopeOwner(Element& element, KeyboardEvent* event)
     303{
     304    return element.isKeyboardFocusable(event) && isFocusScopeOwner(element);
     305}
     306
     307static inline int shadowAdjustedTabIndex(Element& element, KeyboardEvent* event)
    308308{
    309309    if (isNonFocusableScopeOwner(element, event)) {
    … …  
    505505Element* FocusController::findFocusableElementAcrossFocusScope(FocusDirection direction, const FocusNavigationScope& scope, Node* currentNode, KeyboardEvent* event)
    506506{
    507     ASSERT(!is<Element>(currentNode) || !isNonFocusableScopeOwner(downcast<Element>(*currentNode), *event));
    508 
    509     if (currentNode && direction == FocusDirectionForward && is<Element>(currentNode) && isFocusableScopeOwner(downcast<Element>(*currentNode), *event)) {
     507    ASSERT(!is<Element>(currentNode) || !isNonFocusableScopeOwner(downcast<Element>(*currentNode), event));
     508
     509    if (currentNode && direction == FocusDirectionForward && is<Element>(currentNode) && isFocusableScopeOwner(downcast<Element>(*currentNode), event)) {
    510510        if (Element* candidateInInnerScope = findFocusableElementWithinScope(direction, FocusNavigationScope::scopeOwnedByScopeOwner(downcast<Element>(*currentNode)), 0, event))
    511511            return candidateInInnerScope;
    … …  
    518518    Element* owner = scope.owner();
    519519    while (owner) {
    520         if (direction == FocusDirectionBackward && isFocusableScopeOwner(*owner, *event))
     520        if (direction == FocusDirectionBackward && isFocusableScopeOwner(*owner, event))
    521521            return findFocusableElementDescendingDownIntoFrameDocument(direction, owner, event);
    522522
    … …  
    543543    if (!found)
    544544        return nullptr;
    545     if (isNonFocusableScopeOwner(*found, *event)) {
     545    if (isNonFocusableScopeOwner(*found, event)) {
    546546        if (Element* foundInInnerFocusScope = nextFocusableElementWithinScope(FocusNavigationScope::scopeOwnedByScopeOwner(*found), 0, event))
    547547            return foundInInnerFocusScope;
    … …  
    556556    if (!found)
    557557        return nullptr;
    558     if (isFocusableScopeOwner(*found, *event)) {
     558    if (isFocusableScopeOwner(*found, event)) {
    559559        // Search an inner focusable element in the shadow tree from the end.
    560560        if (Element* foundInInnerFocusScope = previousFocusableElementWithinScope(FocusNavigationScope::scopeOwnedByScopeOwner(*found), 0, event))
    … …  
    562562        return found;
    563563    }
    564     if (isNonFocusableScopeOwner(*found, *event)) {
     564    if (isNonFocusableScopeOwner(*found, event)) {
    565565        if (Element* foundInInnerFocusScope = previousFocusableElementWithinScope(FocusNavigationScope::scopeOwnedByScopeOwner(*found), 0, event))
    566566            return foundInInnerFocusScope;
    … …  
    584584            continue;
    585585        Element& element = downcast<Element>(*node);
    586         if (isFocusableElementOrScopeOwner(element, *event) && shadowAdjustedTabIndex(element, *event) == tabIndex)
     586        if (isFocusableElementOrScopeOwner(element, event) && shadowAdjustedTabIndex(element, event) == tabIndex)
    587587            return &element;
    588588    }
    … …  
    590590}
    591591
    592 static Element* nextElementWithGreaterTabIndex(const FocusNavigationScope& scope, int tabIndex, KeyboardEvent& event)
     592static Element* nextElementWithGreaterTabIndex(const FocusNavigationScope& scope, int tabIndex, KeyboardEvent* event)
    593593{
    594594    // Search is inclusive of start
    … …  
    609609}
    610610
    611 static Element* previousElementWithLowerTabIndex(const FocusNavigationScope& scope, Node* start, int tabIndex, KeyboardEvent& event)
     611static Element* previousElementWithLowerTabIndex(const FocusNavigationScope& scope, Node* start, int tabIndex, KeyboardEvent* event)
    612612{
    613613    // Search is inclusive of start
    … …  
    645645    int startTabIndex = 0;
    646646    if (start && is<Element>(*start))
    647         startTabIndex = shadowAdjustedTabIndex(downcast<Element>(*start), *event);
     647        startTabIndex = shadowAdjustedTabIndex(downcast<Element>(*start), event);
    648648
    649649    if (start) {
    … …  
    654654                    continue;
    655655                Element& element = downcast<Element>(*node);
    656                 if (isFocusableElementOrScopeOwner(element, *event) && shadowAdjustedTabIndex(element, *event) >= 0)
     656                if (isFocusableElementOrScopeOwner(element, event) && shadowAdjustedTabIndex(element, event) >= 0)
    657657                    return &element;
    658658            }
    … …  
    670670    // 1) has the lowest tabindex that is higher than start's tabindex (or 0, if start is null), and
    671671    // 2) comes first in the scope, if there's a tie.
    672     if (Element* winner = nextElementWithGreaterTabIndex(scope, startTabIndex, *event))
     672    if (Element* winner = nextElementWithGreaterTabIndex(scope, startTabIndex, event))
    673673        return winner;
    674674
    … …  
    692692        startingNode = scope.previousInScope(start);
    693693        if (is<Element>(*start))
    694             startingTabIndex = shadowAdjustedTabIndex(downcast<Element>(*start), *event);
     694            startingTabIndex = shadowAdjustedTabIndex(downcast<Element>(*start), event);
    695695    } else
    696696        startingNode = last;
    … …  
    702702                continue;
    703703            Element& element = downcast<Element>(*node);
    704             if (isFocusableElementOrScopeOwner(element, *event) && shadowAdjustedTabIndex(element, *event) >= 0)
     704            if (isFocusableElementOrScopeOwner(element, event) && shadowAdjustedTabIndex(element, event) >= 0)
    705705                return &element;
    706706        }
    … …  
    714714    // 2) comes last in the scope, if there's a tie.
    715715    startingTabIndex = (start && startingTabIndex) ? startingTabIndex : std::numeric_limits<int>::max();
    716     return previousElementWithLowerTabIndex(scope, last, startingTabIndex, *event);
     716    return previousElementWithLowerTabIndex(scope, last, startingTabIndex, event);
    717717}
    718718

  • trunk/Source/WebKit2/ChangeLog

    r204916 r204941  
     12016-08-24  Jonathan Bedard  <jbedard@apple.com>
     2
     3        FocusController multiple dereferenced NULL pointers
     4        https://bugs.webkit.org/show_bug.cgi?id=160808
     5
     6        Reviewed by Darin Adler.
     7
     8        * WebProcess/WebPage/WebPage.cpp:
     9        (WebKit::WebPage::setInitialFocus): Should use nullptr, not 0 to initialize NULL pointer.
     10
    1112016-08-23  Anders Carlsson  <andersca@apple.com>
    212

  • trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp

    r204852 r204941  
    25202520    }
    25212521
    2522     m_page->focusController().setInitialFocus(forward ? FocusDirectionForward : FocusDirectionBackward, 0);
     2522    m_page->focusController().setInitialFocus(forward ? FocusDirectionForward : FocusDirectionBackward, nullptr);
    25232523    send(Messages::WebPageProxy::VoidCallback(callbackID));
    25242524}
Note: See TracChangeset for help on using the changeset viewer.