Fix dictionary leak in lookup, convert FindOptions to OptionSet, tweak code style...
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 25 Nov 2017 01:09:58 +0000 (01:09 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 25 Nov 2017 01:09:58 +0000 (01:09 +0000)
commitf826477031ba0b8be03431f48bd85bf64535fe53
tree979543408e1db2c1663194d5ff5f839870116b67
parent4de26bd1a84ac29345f35c6a9724dda161db638b
Fix dictionary leak in lookup, convert FindOptions to OptionSet, tweak code style nearby
https://bugs.webkit.org/show_bug.cgi?id=179981

Reviewed by Sam Weinig.

Source/WebCore:

* accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::rangeMatchesTextNearRange): Pass { } instead of 0.
* accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::rangeOfStringClosestToRangeInDirection const): Use { }
instead of | to construct value.
* editing/Editor.cpp:
(WebCore::Editor::findString): Use contains instead of & to test an option.
(WebCore::Editor::rangeOfString): Ditto.
(WebCore::Editor::countMatchesForText): Ditto.
* editing/EditorCommand.cpp:
(WebCore::executeFindString): Use { } instead of | to construct value.
* editing/FindOptions.h: Use OptionSet for type.
* editing/TextIterator.cpp:
(WebCore::SearchBuffer::SearchBuffer): Use contains instead of &. Also use -= instead
of &= coupled with ~ to remove a flag.
(WebCore::SearchBuffer::isWordEndMatch const): Ditto.
(WebCore::SearchBuffer::isWordStartMatch const): Ditto.
(WebCore::SearchBuffer::search): Ditto.
(WebCore::findIteratorOptions): Ditto.
(WebCore::findClosestPlainText): Ditto.
(WebCore::findPlainText): Ditto.

* editing/mac/DictionaryLookup.h: Changed optional dictionary out arguments to
RetainPtr<NSDictionary>. Before, they were returning a retained object, but callers
did not seem to realize that.
* editing/mac/DictionaryLookup.mm:
(WebCore::tokenRange): Added helper. Takes care of doing adoptNS on the options
dictionary, and also handles the Objective-C exception possibility cleanly. Also
fixed the exception case to return NSNotFound instead of a zero-length range at
the location we are searching, which is what the code expects.
(WebCore::DictionaryLookup::rangeForSelection): Refactored to use the tokenRange
helper function, and also to do much less work when the options pointer is null.
(WebCore::DictionaryLookup::rangeAtHitTestResult): Use the tokenRange helper
function and also use auto a bunch to tighten up the code a bit.
(WebCore::DictionaryLookup::stringForPDFSelection): Ditto.

* page/DOMWindow.cpp:
(WebCore::DOMWindow::find const): Use |= instead of | to build find options.
* page/Page.cpp:
(WebCore::Page::findString): Use contains instead of & and also - instead of
& combined with ~.
(WebCore::Page::findStringMatchingRanges): Ditto.
(WebCore::Page::rangeOfString): Ditto.

* page/mac/EventHandlerMac.mm:
(WebCore::EventHandler::selectClosestWordFromHitTestResultBasedOnLookup):
Pass nullptr to indicate we don't need the options dictionary. The code before
was passing a pointer to a null, which meant we would do the work to get the
options dictionary, not use it, and also leak it.

* testing/Internals.cpp:
(WebCore::Internals::rangeOfStringNearLocation): Pass { } instead of 0.
(WebCore::Internals::rangeForDictionaryLookupAtLocation): Pass nullptr to
indicate we don't need the options dictionary as above.
(WebCore::parseFindOptions): Initialize options without an explicit 0, since
an OptionSet starts out empty rather than uninitialized.

Source/WebKit:

* WebProcess/Plugins/PDF/PDFPlugin.h: Use some references instead of pointers. Made more
things final and private. Changed functions with multiple return values to use tuples
instead of out arguments for clarity, especially because some were using pointers and it
was ambiguous whether the pointers could be nullptr. Tweaked formatting. Moved initialization
of data members into the class definition.

* WebProcess/Plugins/PDF/PDFPlugin.mm:
(WebKit::PDFPlugin::create): Take a reference.
(WebKit::PDFPlugin::PDFPlugin): Ditto. Also moved some initialization to the class definition.
(WebKit::PDFPlugin::countFindMatches): Added comment about ignored maxMatchCount argument.
(WebKit::PDFPlugin::findString): Use contains instead of & to check bits in options.
Simplified slightly confusing match count code that was doing some unnecessary comparisons
with the maximum match count. Use auto a bit.
(WebKit::coreCursor): Renamed from pdfLayerControllerCursorTypeToCursor since this is C++
and overloading works based on the argument type.
(WebKit::PDFPlugin::notifyCursorChanged): Updated for name change.
(WebKit::PDFPlugin::lookupTextAtLocation const): Return a tuple instead of using two out
arguments and use a RetainPtr so we don't leak the options dictionary.

* WebProcess/Plugins/PluginProxy.h: Removed unneeded include of FindOptions.h.

* WebProcess/WebCoreSupport/mac/WebContextMenuClientMac.mm:
(WebKit::WebContextMenuClient::lookUpInDictionary): Pass a reference.

* WebProcess/WebPage/FindController.cpp:
(WebKit::core): Use |= instead of | to build up a FindOptions.
(WebKit::FindController::FindController): Initialize data members in the class definition.
(WebKit::pluginViewForFrame): Deleted. Callers now use WebPage::pluginViewForFrame.
(WebKit::FindController::updateFindUIAfterPageScroll): Added a FIXME about some peculiar code.
(WebKit::FindController::findString): Use |= rather than | to add in an option.
(WebKit::FindController::hideFindUI): Use { } rather than 0 for no options.
* WebProcess/WebPage/FindController.h: Moved initialization to the header. Exported the core
function that converts WebKit::FindOptions to WebCore::FindOptions.

* WebProcess/WebPage/WebFrame.cpp:
(WebKit::WebFrame::handlesPageScaleGesture const): Use WebPage::pluginViewForFrame.
(WebKit::WebFrame::requiresUnifiedScaleFactor const): Use WebPage::pluginViewForFrame.

* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::createPlugin): Removed unneeded local variable and UNUSED_PARAM.
(WebKit::WebPage::focusedPluginViewForFrame): Use pluginViewForFrame.
(WebKit::WebPage::pluginViewForFrame): Added a null check since this takes a pointer.
Use is/downcast insteadof more ad hoc coding style.
(WebKit::WebPage::findStringFromInjectedBundle): Call core to convert WebKit::FindOptions
to WebCore::FindOptions. Before, this was accidentally relying on the bits from the two
enumrations matching! The stricter type checking of OptionSet helps us catch mistakes
like this.

* WebProcess/WebPage/WebPage.h: Updated for the above. Also flattened out nested #if
statements, made forward declarations unconditional, and re-sorted them. Changed the
Mac-specific lookupTextAtLocation to return a tuple (see below).

* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::performDictionaryLookupForSelection): Take a reference instead of a pointer.
(WebKit::WebPage::performDictionaryLookupForRange): Ditto.
(WebKit::rangeNearPositionMatchesText): Use { } rather than 0 for no options.

* WebProcess/WebPage/mac/WebPageMac.mm:
(WebKit::WebPage::performDictionaryLookupAtLocation): Use RetainPtr<NSDictionary> to fix
code that used to leak.
(WebKit::WebPage::performDictionaryLookupForSelection): Ditto.
(WebKit::WebPage::performDictionaryLookupOfCurrentSelection): Use a reference.
(WebKit::WebPage::dictionaryPopupInfoForRange): Updated for changed argument types.
(WebKit::WebPage::dictionaryPopupInfoForSelectionInPDFPlugin): Ditto.
(WebKit::WebPage::performDictionaryLookupForRange): Ditto.
(WebKit::WebPage::performImmediateActionHitTestAtLocation): Updated to handle the
tuple result from the lookupTextAtLocation functions.
(WebKit::WebPage::lookupTextAtLocation): Changed to return a tuple and use RetainPtr
for the NSDictionary to help fix the leak.

Source/WebKitLegacy/mac:

* WebView/WebImmediateActionController.mm:
(-[WebImmediateActionController _animationControllerForText]): Use RetainPtr so we don't leak.
* WebView/WebView.mm:
(coreOptions): Use |= instead of | to build up FindOptions.

Source/WebKitLegacy/win:

* WebView.cpp:
(WebView::searchFor): Use |= instead of | to build FindOptions.
(WebView::markAllMatchesForText): Create FindOptions with |= instead of |.
(WebView::findString): Create FindOptions with |=; the old code just passed a
WebKit FindOptions through without converting to WebCore::FindOptions.

Source/WTF:

* wtf/OptionSet.h: Added some operators so it's easier to use | and - with individual
options from the set.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@225142 268f45cc-cd09-0410-ab3c-d52691b4dbfc
34 files changed:
Source/WTF/ChangeLog
Source/WTF/wtf/OptionSet.h
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AXObjectCache.cpp
Source/WebCore/accessibility/AccessibilityObject.cpp
Source/WebCore/editing/Editor.cpp
Source/WebCore/editing/EditorCommand.cpp
Source/WebCore/editing/FindOptions.h
Source/WebCore/editing/TextIterator.cpp
Source/WebCore/editing/mac/DictionaryLookup.h
Source/WebCore/editing/mac/DictionaryLookup.mm
Source/WebCore/page/DOMWindow.cpp
Source/WebCore/page/Page.cpp
Source/WebCore/page/mac/EventHandlerMac.mm
Source/WebCore/testing/Internals.cpp
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.h
Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm
Source/WebKit/WebProcess/Plugins/PluginProxy.h
Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
Source/WebKit/WebProcess/WebCoreSupport/mac/WebContextMenuClientMac.mm
Source/WebKit/WebProcess/WebPage/FindController.cpp
Source/WebKit/WebProcess/WebPage/FindController.h
Source/WebKit/WebProcess/WebPage/WebFrame.cpp
Source/WebKit/WebProcess/WebPage/WebPage.cpp
Source/WebKit/WebProcess/WebPage/WebPage.h
Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
Source/WebKit/WebProcess/WebPage/mac/WKAccessibilityWebPageObjectMac.mm
Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm
Source/WebKitLegacy/mac/ChangeLog
Source/WebKitLegacy/mac/WebView/WebImmediateActionController.mm
Source/WebKitLegacy/mac/WebView/WebView.mm
Source/WebKitLegacy/win/ChangeLog
Source/WebKitLegacy/win/WebView.cpp