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)
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

index c13ecb4..4f670f3 100644 (file)
@@ -1,5 +1,15 @@
 2017-11-23  Darin Adler  <darin@apple.com>
 
+        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.
+
+        * wtf/OptionSet.h: Added some operators so it's easier to use | and - with individual
+        options from the set.
+
+2017-11-23  Darin Adler  <darin@apple.com>
+
         Reduce WTF::String operations that do unnecessary Unicode operations instead of ASCII
         https://bugs.webkit.org/show_bug.cgi?id=179907
 
index f3a8c00..d063b5b 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -121,20 +121,33 @@ public:
     friend OptionSet& operator|=(OptionSet& lhs, OptionSet rhs)
     {
         lhs.m_storage |= rhs.m_storage;
-
         return lhs;
     }
 
     friend OptionSet& operator-=(OptionSet& lhs, OptionSet rhs)
     {
         lhs.m_storage &= ~rhs.m_storage;
-
         return lhs;
     }
 
+    constexpr friend OptionSet operator|(OptionSet lhs, OptionSet rhs)
+    {
+        return fromRaw(lhs.m_storage | rhs.m_storage);
+    }
+
+    constexpr friend OptionSet operator|(OptionSet lhs, T rhs)
+    {
+        return lhs | OptionSet { rhs };
+    }
+
     constexpr friend OptionSet operator-(OptionSet lhs, OptionSet rhs)
     {
-        return OptionSet::fromRaw(lhs.m_storage & ~rhs.m_storage);
+        return fromRaw(lhs.m_storage & ~rhs.m_storage);
+    }
+
+    constexpr friend OptionSet operator-(OptionSet lhs, T rhs)
+    {
+        return lhs - OptionSet { rhs };
     }
 
 private:
index 3f477d5..101d035 100644 (file)
@@ -1,3 +1,67 @@
+2017-11-23  Darin Adler  <darin@apple.com>
+
+        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.
+
+        * 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.
+
 2017-11-24  Antti Koivisto  <antti@apple.com>
 
         Style resolution spin due to calc() values always comparing inequal (seen on arstechnica.com)
index 1df1f05..6eadaf4 100644 (file)
@@ -1736,7 +1736,7 @@ RefPtr<Range> AXObjectCache::rangeMatchesTextNearRange(RefPtr<Range> originalRan
     
     RefPtr<Range> range = Range::create(m_document, startPosition, originalRange->startPosition());
     unsigned targetOffset = TextIterator::rangeLength(range.get(), true);
-    return findClosestPlainText(*searchRange.get(), matchText, 0, targetOffset);
+    return findClosestPlainText(*searchRange.get(), matchText, { }, targetOffset);
 }
 
 static bool isReplacedNodeOrBR(Node* node)
index bd4eb81..a5ac285 100644 (file)
@@ -748,7 +748,7 @@ RefPtr<Range> AccessibilityObject::rangeOfStringClosestToRangeInDirection(Range*
         return nullptr;
     
     bool isBackwardSearch = searchDirection == AccessibilitySearchDirection::Previous;
-    FindOptions findOptions = AtWordStarts | AtWordEnds | CaseInsensitive | StartInSelection;
+    FindOptions findOptions { { AtWordStarts, AtWordEnds, CaseInsensitive, StartInSelection } };
     if (isBackwardSearch)
         findOptions |= Backwards;
     
index 8a997e4..8be0411 100644 (file)
@@ -3245,7 +3245,7 @@ bool Editor::findString(const String& target, FindOptions options)
 
     m_frame.selection().setSelection(VisibleSelection(*resultRange, DOWNSTREAM));
 
-    if (!(options & DoNotRevealSelection))
+    if (!(options.contains(DoNotRevealSelection)))
         m_frame.selection().revealSelection();
 
     return true;
@@ -3260,8 +3260,8 @@ RefPtr<Range> Editor::rangeOfString(const String& target, Range* referenceRange,
     // is used depends on whether we're searching forward or backward, and whether startInSelection is set.
     RefPtr<Range> searchRange(rangeOfContents(document()));
 
-    bool forward = !(options & Backwards);
-    bool startInReferenceRange = referenceRange && (options & StartInSelection);
+    bool forward = !options.contains(Backwards);
+    bool startInReferenceRange = referenceRange && options.contains(StartInSelection);
     if (referenceRange) {
         if (forward)
             searchRange->setStart(startInReferenceRange ? referenceRange->startPosition() : referenceRange->endPosition());
@@ -3313,7 +3313,7 @@ RefPtr<Range> Editor::rangeOfString(const String& target, Range* referenceRange,
 
     // If we didn't find anything and we're wrapping, search again in the entire document (this will
     // redundantly re-search the area already searched in some cases).
-    if (resultRange->collapsed() && options & WrapAround) {
+    if (resultRange->collapsed() && options.contains(WrapAround)) {
         searchRange = rangeOfContents(document());
         resultRange = findPlainText(*searchRange, target, options);
         // We used to return false here if we ended up with the same range that we started with
@@ -3355,7 +3355,7 @@ unsigned Editor::countMatchesForText(const String& target, Range* range, FindOpt
 
     unsigned matchCount = 0;
     do {
-        RefPtr<Range> resultRange(findPlainText(*searchRange, target, options & ~Backwards));
+        RefPtr<Range> resultRange(findPlainText(*searchRange, target, options Backwards));
         if (resultRange->collapsed()) {
             if (!resultRange->startContainer().isInShadowTree())
                 break;
index 9ec1f0d..94d8692 100644 (file)
@@ -378,7 +378,7 @@ static bool executeDeleteWordForward(Frame& frame, Event*, EditorCommandSource,
 
 static bool executeFindString(Frame& frame, Event*, EditorCommandSource, const String& value)
 {
-    return frame.editor().findString(value, CaseInsensitive | WrapAround | DoNotTraverseFlatTree);
+    return frame.editor().findString(value, { CaseInsensitive, WrapAround, DoNotTraverseFlatTree });
 }
 
 static bool executeFontName(Frame& frame, Event*, EditorCommandSource source, const String& value)
index bf735ec..e71de7a 100644 (file)
@@ -25,6 +25,8 @@
 
 #pragma once
 
+#include <wtf/OptionSet.h>
+
 namespace WebCore {
 
 enum FindOptionFlag {
@@ -41,6 +43,6 @@ enum FindOptionFlag {
     DoNotTraverseFlatTree = 1 << 8,
 };
 
-typedef unsigned short FindOptions;
+using FindOptions = OptionSet<FindOptionFlag>;
 
 } // namespace WebCore
index 3af1acf..6b3fb9f 100644 (file)
@@ -2061,7 +2061,7 @@ inline SearchBuffer::SearchBuffer(const String& target, FindOptions options)
     , m_options(options)
     , m_prefixLength(0)
     , m_atBreak(true)
-    , m_needsMoreContext(options & AtWordStarts)
+    , m_needsMoreContext(options.contains(AtWordStarts))
     , m_targetRequiresKanaWorkaround(containsKanaLetters(m_target))
 {
     ASSERT(!m_target.isEmpty());
@@ -2070,13 +2070,13 @@ inline SearchBuffer::SearchBuffer(const String& target, FindOptions options)
     m_buffer.reserveInitialCapacity(std::max(targetLength * 8, minimumSearchBufferSize));
     m_overlap = m_buffer.capacity() / 4;
 
-    if ((m_options & AtWordStarts) && targetLength) {
+    if (m_options.contains(AtWordStarts) && targetLength) {
         UChar32 targetFirstCharacter;
         U16_GET(m_target, 0, 0u, targetLength, targetFirstCharacter);
         // Characters in the separator category never really occur at the beginning of a word,
         // so if the target begins with such a character, we just ignore the AtWordStart option.
         if (isSeparator(targetFirstCharacter)) {
-            m_options &= ~AtWordStarts;
+            m_options -= AtWordStarts;
             m_needsMoreContext = false;
         }
     }
@@ -2091,7 +2091,7 @@ inline SearchBuffer::SearchBuffer(const String& target, FindOptions options)
 
     UCollationStrength strength;
     USearchAttributeValue comparator;
-    if (m_options & CaseInsensitive) {
+    if (m_options.contains(CaseInsensitive)) {
         // Without loss of generality, have 'e' match {'e', 'E', 'é', 'É'} and 'é' match {'é', 'É'}.
         strength = UCOL_SECONDARY;
         comparator = USEARCH_PATTERN_BASE_WEIGHT_IS_WILDCARD;
@@ -2254,7 +2254,7 @@ inline bool SearchBuffer::isBadMatch(const UChar* match, size_t matchLength) con
 inline bool SearchBuffer::isWordEndMatch(size_t start, size_t length) const
 {
     ASSERT(length);
-    ASSERT(m_options & AtWordEnds);
+    ASSERT(m_options.contains(AtWordEnds));
 
     int endWord;
     // Start searching at the end of matched search, so that multiple word matches succeed.
@@ -2264,7 +2264,7 @@ inline bool SearchBuffer::isWordEndMatch(size_t start, size_t length) const
 
 inline bool SearchBuffer::isWordStartMatch(size_t start, size_t length) const
 {
-    ASSERT(m_options & AtWordStarts);
+    ASSERT(m_options.contains(AtWordStarts));
 
     if (!start)
         return true;
@@ -2274,7 +2274,7 @@ inline bool SearchBuffer::isWordStartMatch(size_t start, size_t length) const
     UChar32 firstCharacter;
     U16_GET(m_buffer.data(), 0, offset, size, firstCharacter);
 
-    if (m_options & TreatMedialCapitalAsWordStart) {
+    if (m_options.contains(TreatMedialCapitalAsWordStart)) {
         UChar32 previousCharacter;
         U16_PREV(m_buffer.data(), 0, offset, previousCharacter);
 
@@ -2351,7 +2351,7 @@ nextMatch:
     // possibly including a combining character that's not yet in the buffer.
     if (!m_atBreak && static_cast<size_t>(matchStart) >= size - m_overlap) {
         size_t overlap = m_overlap;
-        if (m_options & AtWordStarts) {
+        if (m_options.contains(AtWordStarts)) {
             // Ensure that there is sufficient context before matchStart the next time around for
             // determining if it is at a word boundary.
             unsigned wordBoundaryContextStart = matchStart;
@@ -2370,8 +2370,8 @@ nextMatch:
 
     // If this match is "bad", move on to the next match.
     if (isBadMatch(m_buffer.data() + matchStart, matchedLength)
-        || ((m_options & AtWordStarts) && !isWordStartMatch(matchStart, matchedLength))
-        || ((m_options & AtWordEnds) && !isWordEndMatch(matchStart, matchedLength))) {
+        || (m_options.contains(AtWordStarts) && !isWordStartMatch(matchStart, matchedLength))
+        || (m_options.contains(AtWordEnds) && !isWordEndMatch(matchStart, matchedLength))) {
         matchStart = usearch_next(searcher, &status);
         ASSERT(status == U_ZERO_ERROR);
         goto nextMatch;
@@ -2681,7 +2681,7 @@ static Ref<Range> collapsedToBoundary(const Range& range, bool forward)
 static TextIteratorBehavior findIteratorOptions(FindOptions options)
 {
     TextIteratorBehavior iteratorOptions = TextIteratorEntersTextControls | TextIteratorClipsToFrameAncestors;
-    if (!(options & DoNotTraverseFlatTree))
+    if (!options.contains(DoNotTraverseFlatTree))
         iteratorOptions |= TextIteratorTraversesFlatTree;
     return iteratorOptions;
 }
@@ -2744,12 +2744,12 @@ Ref<Range> findClosestPlainText(const Range& range, const String& target, FindOp
     };
 
     findPlainTextMatches(range, target, options, WTFMove(match));
-    return rangeForMatch(range, options, matchStart, matchLength, !(options & Backwards));
+    return rangeForMatch(range, options, matchStart, matchLength, !options.contains(Backwards));
 }
 
 Ref<Range> findPlainText(const Range& range, const String& target, FindOptions options)
 {
-    bool searchForward = !(options & Backwards);
+    bool searchForward = !options.contains(Backwards);
     size_t matchStart = 0;
     size_t matchLength = 0;
     auto match = [searchForward, &matchStart, &matchLength] (size_t start, size_t length) {
index d92739f..908e56b 100644 (file)
@@ -50,9 +50,9 @@ class VisibleSelection;
 
 class DictionaryLookup {
 public:
-    WEBCORE_EXPORT static RefPtr<Range> rangeForSelection(const VisibleSelection&, NSDictionary **options);
-    WEBCORE_EXPORT static RefPtr<Range> rangeAtHitTestResult(const HitTestResult&, NSDictionary **options);
-    WEBCORE_EXPORT static NSString *stringForPDFSelection(PDFSelection *, NSDictionary **options);
+    WEBCORE_EXPORT static RefPtr<Range> rangeForSelection(const VisibleSelection&, RetainPtr<NSDictionary> *options);
+    WEBCORE_EXPORT static RefPtr<Range> rangeAtHitTestResult(const HitTestResult&, RetainPtr<NSDictionary> *options);
+    WEBCORE_EXPORT static NSString *stringForPDFSelection(PDFSelection *, RetainPtr<NSDictionary> *options);
 
     // FIXME: Should move/unify dictionaryPopupInfoForRange here too.
 
index 28f72ff..57a8f3b 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014 Apple Inc. All rights reserved.
+ * Copyright (C) 2014-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -52,6 +52,26 @@ SOFT_LINK_CONSTANT_MAY_FAIL(Lookup, LUTermOptionDisableSearchTermIndicator, NSSt
 
 namespace WebCore {
 
+static NSRange tokenRange(const String& string, NSRange range, RetainPtr<NSDictionary>* options)
+{
+    if (!getLULookupDefinitionModuleClass())
+        return NSMakeRange(NSNotFound, 0);
+
+    BEGIN_BLOCK_OBJC_EXCEPTIONS;
+
+    if (!options)
+        return [classLULookupDefinitionModule tokenRangeForString:string range:range options:nullptr];
+
+    NSDictionary *retainedOptions = nil;
+    auto result = [classLULookupDefinitionModule tokenRangeForString:string range:range options:&retainedOptions];
+    *options = adoptNS(retainedOptions);
+    return result;
+
+    END_BLOCK_OBJC_EXCEPTIONS;
+
+    return NSMakeRange(NSNotFound, 0);
+}
+
 static bool selectionContainsPosition(const VisiblePosition& position, const VisibleSelection& selection)
 {
     if (!selection.isRange())
@@ -64,90 +84,73 @@ static bool selectionContainsPosition(const VisiblePosition& position, const Vis
     return selectedRange->contains(position);
 }
 
-RefPtr<Range> DictionaryLookup::rangeForSelection(const VisibleSelection& selection, NSDictionary **options)
+RefPtr<Range> DictionaryLookup::rangeForSelection(const VisibleSelection& selection, RetainPtr<NSDictionary>* options)
 {
-    RefPtr<Range> selectedRange = selection.toNormalizedRange();
+    auto selectedRange = selection.toNormalizedRange();
     if (!selectedRange)
         return nullptr;
 
-    VisiblePosition selectionStart = selection.visibleStart();
-    VisiblePosition selectionEnd = selection.visibleEnd();
-
-    // As context, we are going to use the surrounding paragraphs of text.
-    VisiblePosition paragraphStart = startOfParagraph(selectionStart);
-    VisiblePosition paragraphEnd = endOfParagraph(selectionEnd);
+    // Since we already have the range we want, we just need to grab the returned options.
+    if (options) {
+        auto selectionStart = selection.visibleStart();
+        auto selectionEnd = selection.visibleEnd();
 
-    int lengthToSelectionStart = TextIterator::rangeLength(makeRange(paragraphStart, selectionStart).get());
-    int lengthToSelectionEnd = TextIterator::rangeLength(makeRange(paragraphStart, selectionEnd).get());
-    NSRange rangeToPass = NSMakeRange(lengthToSelectionStart, lengthToSelectionEnd - lengthToSelectionStart);
+        // As context, we are going to use the surrounding paragraphs of text.
+        auto paragraphStart = startOfParagraph(selectionStart);
+        auto paragraphEnd = endOfParagraph(selectionEnd);
 
-    String fullPlainTextString = plainText(makeRange(paragraphStart, paragraphEnd).get());
+        int lengthToSelectionStart = TextIterator::rangeLength(makeRange(paragraphStart, selectionStart).get());
+        int lengthToSelectionEnd = TextIterator::rangeLength(makeRange(paragraphStart, selectionEnd).get());
+        NSRange rangeToPass = NSMakeRange(lengthToSelectionStart, lengthToSelectionEnd - lengthToSelectionStart);
 
-    BEGIN_BLOCK_OBJC_EXCEPTIONS;
-    // Since we already have the range we want, we just need to grab the returned options.
-    if (Class luLookupDefinitionModule = getLULookupDefinitionModuleClass())
-        [luLookupDefinitionModule tokenRangeForString:fullPlainTextString range:rangeToPass options:options];
-    END_BLOCK_OBJC_EXCEPTIONS;
+        tokenRange(plainText(makeRange(paragraphStart, paragraphEnd).get()), rangeToPass, options);
+    }
 
     return selectedRange;
 }
 
-RefPtr<Range> DictionaryLookup::rangeAtHitTestResult(const HitTestResult& hitTestResult, NSDictionary **options)
+RefPtr<Range> DictionaryLookup::rangeAtHitTestResult(const HitTestResult& hitTestResult, RetainPtr<NSDictionary>* options)
 {
-    Node* node = hitTestResult.innerNonSharedNode();
-    if (!node)
-        return nullptr;
-
-    auto renderer = node->renderer();
-    if (!renderer)
+    auto* node = hitTestResult.innerNonSharedNode();
+    if (!node || !node->renderer())
         return nullptr;
 
-    Frame* frame = node->document().frame();
+    auto* frame = node->document().frame();
     if (!frame)
         return nullptr;
 
     // Don't do anything if there is no character at the point.
-    IntPoint framePoint = hitTestResult.roundedPointInInnerNodeFrame();
+    auto framePoint = hitTestResult.roundedPointInInnerNodeFrame();
     if (!frame->rangeForPoint(framePoint))
         return nullptr;
 
-    VisiblePosition position = frame->visiblePositionForPoint(framePoint);
+    auto position = frame->visiblePositionForPoint(framePoint);
     if (position.isNull())
         position = firstPositionInOrBeforeNode(node);
 
     // If we hit the selection, use that instead of letting Lookup decide the range.
-    VisibleSelection selection = frame->page()->focusController().focusedOrMainFrame().selection().selection();
+    auto selection = frame->page()->focusController().focusedOrMainFrame().selection().selection();
     if (selectionContainsPosition(position, selection))
-        return DictionaryLookup::rangeForSelection(selection, options);
+        return rangeForSelection(selection, options);
 
-    VisibleSelection selectionAccountingForLineRules = VisibleSelection(position);
+    VisibleSelection selectionAccountingForLineRules { position };
     selectionAccountingForLineRules.expandUsingGranularity(WordGranularity);
     position = selectionAccountingForLineRules.start();
 
     // As context, we are going to use 250 characters of text before and after the point.
-    RefPtr<Range> fullCharacterRange = rangeExpandedAroundPositionByCharacters(position, 250);
+    auto fullCharacterRange = rangeExpandedAroundPositionByCharacters(position, 250);
     if (!fullCharacterRange)
         return nullptr;
 
-    BEGIN_BLOCK_OBJC_EXCEPTIONS;
-
     NSRange rangeToPass = NSMakeRange(TextIterator::rangeLength(makeRange(fullCharacterRange->startPosition(), position).get()), 0);
+    NSRange extractedRange = tokenRange(plainText(fullCharacterRange.get()), rangeToPass, options);
 
-    String fullPlainTextString = plainText(fullCharacterRange.get());
-
-    NSRange extractedRange = NSMakeRange(rangeToPass.location, 0);
-    if (Class luLookupDefinitionModule = getLULookupDefinitionModuleClass())
-        extractedRange = [luLookupDefinitionModule tokenRangeForString:fullPlainTextString range:rangeToPass options:options];
-
-    // This function sometimes returns {NSNotFound, 0} if it was unable to determine a good string.
+    // tokenRange sometimes returns {NSNotFound, 0} if it was unable to determine a good string.
     // FIXME (159063): We shouldn't need to check for zero length here.
     if (extractedRange.location == NSNotFound || extractedRange.length == 0)
         return nullptr;
 
     return TextIterator::subrange(*fullCharacterRange, extractedRange.location, extractedRange.length);
-
-    END_BLOCK_OBJC_EXCEPTIONS;
-    return nullptr;
 }
 
 static void expandSelectionByCharacters(PDFSelection *selection, NSInteger numberOfCharactersToExpand, NSInteger& charactersAddedBeforeStart, NSInteger& charactersAddedAfterEnd)
@@ -165,43 +168,42 @@ static void expandSelectionByCharacters(PDFSelection *selection, NSInteger numbe
     END_BLOCK_OBJC_EXCEPTIONS;
 }
 
-NSString *DictionaryLookup::stringForPDFSelection(PDFSelection *selection, NSDictionary **options)
+NSString *DictionaryLookup::stringForPDFSelection(PDFSelection *selection, RetainPtr<NSDictionary>* options)
 {
     BEGIN_BLOCK_OBJC_EXCEPTIONS;
 
     // Don't do anything if there is no character at the point.
     if (!selection || !selection.string.length)
         return @"";
-    
+
     RetainPtr<PDFSelection> selectionForLookup = adoptNS([selection copy]);
-    
+
     // As context, we are going to use 250 characters of text before and after the point.
-    NSInteger originalLength = [selectionForLookup string].length;
+    auto originalLength = [selectionForLookup string].length;
     NSInteger charactersAddedBeforeStart = 0;
     NSInteger charactersAddedAfterEnd = 0;
     expandSelectionByCharacters(selectionForLookup.get(), 250, charactersAddedBeforeStart, charactersAddedAfterEnd);
-    
-    NSString *fullPlainTextString = [selectionForLookup string];
-    NSRange rangeToPass = NSMakeRange(charactersAddedBeforeStart, 0);
-    
-    NSRange extractedRange = NSMakeRange(rangeToPass.location, 0);
-    if (Class luLookupDefinitionModule = getLULookupDefinitionModuleClass())
-        extractedRange = [luLookupDefinitionModule tokenRangeForString:fullPlainTextString range:rangeToPass options:options];
-    
+
+    auto fullPlainTextString = [selectionForLookup string];
+    auto rangeToPass = NSMakeRange(charactersAddedBeforeStart, 0);
+
+    auto extractedRange = tokenRange(fullPlainTextString, rangeToPass, options);
+
     // This function sometimes returns {NSNotFound, 0} if it was unable to determine a good string.
     if (extractedRange.location == NSNotFound)
         return selection.string;
-    
+
     NSInteger lookupAddedBefore = rangeToPass.location - extractedRange.location;
     NSInteger lookupAddedAfter = (extractedRange.location + extractedRange.length) - (rangeToPass.location + originalLength);
-    
+
     [selection extendSelectionAtStart:lookupAddedBefore];
     [selection extendSelectionAtEnd:lookupAddedAfter];
-    
+
     ASSERT([selection.string isEqualToString:[fullPlainTextString substringWithRange:extractedRange]]);
     return selection.string;
 
     END_BLOCK_OBJC_EXCEPTIONS;
+
     return nil;
 }
 
index 4de2022..4101ebb 100644 (file)
@@ -1208,7 +1208,13 @@ bool DOMWindow::find(const String& string, bool caseSensitive, bool backwards, b
         return false;
 
     // FIXME (13016): Support wholeWord, searchInFrames and showDialog.    
-    FindOptions options = (backwards ? Backwards : 0) | (caseSensitive ? 0 : CaseInsensitive) | (wrap ? WrapAround : 0);
+    FindOptions options;
+    if (backwards)
+        options |= Backwards;
+    if (!caseSensitive)
+        options |= CaseInsensitive;
+    if (wrap)
+        options |= WrapAround;
     return m_frame->editor().findString(string, options | DoNotTraverseFlatTree);
 }
 
index 814ea67..14c0264 100644 (file)
@@ -586,17 +586,17 @@ bool Page::findString(const String& target, FindOptions options, DidWrap* didWra
     if (target.isEmpty())
         return false;
 
-    CanWrap canWrap = options & WrapAround ? CanWrap::Yes : CanWrap::No;
+    CanWrap canWrap = options.contains(WrapAround) ? CanWrap::Yes : CanWrap::No;
     Frame* frame = &focusController().focusedOrMainFrame();
     Frame* startFrame = frame;
     do {
-        if (frame->editor().findString(target, (options & ~WrapAround) | StartInSelection)) {
+        if (frame->editor().findString(target, (options WrapAround) | StartInSelection)) {
             if (frame != startFrame)
                 startFrame->selection().clear();
             focusController().setFocusedFrame(frame);
             return true;
         }
-        frame = incrementFrame(frame, !(options & Backwards), canWrap, didWrap);
+        frame = incrementFrame(frame, !options.contains(Backwards), canWrap, didWrap);
     } while (frame && frame != startFrame);
 
     // Search contents of startFrame, on the other side of the selection that we did earlier.
@@ -631,7 +631,7 @@ void Page::findStringMatchingRanges(const String& target, FindOptions options, i
     if (frameWithSelection) {
         indexForSelection = NoMatchAfterUserSelection;
         RefPtr<Range> selectedRange = frameWithSelection->selection().selection().firstRange();
-        if (options & Backwards) {
+        if (options.contains(Backwards)) {
             for (size_t i = matchRanges.size(); i > 0; --i) {
                 auto result = selectedRange->compareBoundaryPoints(Range::END_TO_START, *matchRanges[i - 1]);
                 if (!result.hasException() && result.releaseReturnValue() > 0) {
@@ -649,7 +649,7 @@ void Page::findStringMatchingRanges(const String& target, FindOptions options, i
             }
         }
     } else {
-        if (options & Backwards)
+        if (options.contains(Backwards))
             indexForSelection = matchRanges.size() - 1;
         else
             indexForSelection = 0;
@@ -664,14 +664,14 @@ RefPtr<Range> Page::rangeOfString(const String& target, Range* referenceRange, F
     if (referenceRange && referenceRange->ownerDocument().page() != this)
         return nullptr;
 
-    CanWrap canWrap = options & WrapAround ? CanWrap::Yes : CanWrap::No;
+    CanWrap canWrap = options.contains(WrapAround) ? CanWrap::Yes : CanWrap::No;
     Frame* frame = referenceRange ? referenceRange->ownerDocument().frame() : &mainFrame();
     Frame* startFrame = frame;
     do {
-        if (RefPtr<Range> resultRange = frame->editor().rangeOfString(target, frame == startFrame ? referenceRange : 0, options & ~WrapAround))
+        if (RefPtr<Range> resultRange = frame->editor().rangeOfString(target, frame == startFrame ? referenceRange : 0, options WrapAround))
             return resultRange;
 
-        frame = incrementFrame(frame, !(options & Backwards), canWrap);
+        frame = incrementFrame(frame, !options.contains(Backwards), canWrap);
     } while (frame && frame != startFrame);
 
     // Search contents of startFrame, on the other side of the reference range that we did earlier.
index 1731c65..d76563d 100644 (file)
@@ -1090,8 +1090,7 @@ VisibleSelection EventHandler::selectClosestWordFromHitTestResultBasedOnLookup(c
     if (!m_frame.editor().behavior().shouldSelectBasedOnDictionaryLookup())
         return VisibleSelection();
 
-    NSDictionary *options = nil;
-    if (RefPtr<Range> range = DictionaryLookup::rangeAtHitTestResult(result, &options))
+    if (auto range = DictionaryLookup::rangeAtHitTestResult(result, nullptr))
         return VisibleSelection(*range);
 
     return VisibleSelection();
index cd278aa..0c945e6 100644 (file)
@@ -1725,7 +1725,7 @@ Ref<Range> Internals::subrange(Range& range, int rangeLocation, int rangeLength)
 
 RefPtr<Range> Internals::rangeOfStringNearLocation(const Range& searchRange, const String& text, unsigned targetOffset)
 {
-    return findClosestPlainText(searchRange, text, 0, targetOffset);
+    return findClosestPlainText(searchRange, text, { }, targetOffset);
 }
 
 ExceptionOr<RefPtr<Range>> Internals::rangeForDictionaryLookupAtLocation(int x, int y)
@@ -1738,8 +1738,7 @@ ExceptionOr<RefPtr<Range>> Internals::rangeForDictionaryLookupAtLocation(int x,
     document->updateLayoutIgnorePendingStylesheets();
 
     HitTestResult result = document->frame()->mainFrame().eventHandler().hitTestResultAtPoint(IntPoint(x, y));
-    NSDictionary *options = nullptr;
-    return DictionaryLookup::rangeAtHitTestResult(result, &options);
+    return DictionaryLookup::rangeAtHitTestResult(result, nullptr);
 #else
     UNUSED_PARAM(x);
     UNUSED_PARAM(y);
@@ -2119,7 +2118,7 @@ static ExceptionOr<FindOptions> parseFindOptions(const Vector<String>& optionLis
         {"AtWordEnds", AtWordEnds},
         {"DoNotTraverseFlatTree", DoNotTraverseFlatTree},
     };
-    FindOptions result = 0;
+    FindOptions result;
     for (auto& option : optionList) {
         bool found = false;
         for (auto& flag : flagList) {
@@ -2132,7 +2131,7 @@ static ExceptionOr<FindOptions> parseFindOptions(const Vector<String>& optionLis
         if (!found)
             return Exception { SyntaxError };
     }
-    return result;
+    return WTFMove(result);
 }
 
 ExceptionOr<RefPtr<Range>> Internals::rangeOfString(const String& text, RefPtr<Range>&& referenceRange, const Vector<String>& findOptions)
index 581ea8b..e872b23 100644 (file)
@@ -1,3 +1,80 @@
+2017-11-23  Darin Adler  <darin@apple.com>
+
+        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.
+
+        * 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.
+
 2017-11-24  Zan Dobersek  <zdobersek@igalia.com>
 
         [CoordGraphics] CoordinatedGraphicsLayer::updateContentBuffers() should always assume a non-null CoordinatedBuffer
index 4c63a7e..3a44711 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2011, 2012, 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2011-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef PDFPlugin_h
-#define PDFPlugin_h
-
-#include "PDFKitImports.h"
+#pragma once
 
 #if ENABLE(PDFKIT_PLUGIN)
 
+#include "PDFKitImports.h"
 #include "Plugin.h"
 #include "WebEvent.h"
 #include "WebHitTestResultData.h"
@@ -72,7 +70,7 @@ class WebFrame;
 
 class PDFPlugin final : public Plugin, private WebCore::ScrollableArea {
 public:
-    static Ref<PDFPlugin> create(WebFrame*);
+    static Ref<PDFPlugin> create(WebFrame&);
     ~PDFPlugin();
 
     static WebCore::PluginInfo pluginInfo();
@@ -83,7 +81,7 @@ public:
 
     void paintControlForLayerInContext(CALayer *, CGContextRef);
     void setActiveAnnotation(PDFAnnotation *);
-    
+
     using ScrollableArea::notifyScrollPositionChanged;
     void notifyContentScaleFactorChanged(CGFloat scaleFactor);
     void notifyDisplayModeChanged(int);
@@ -107,123 +105,122 @@ public:
     WebCore::FloatRect convertFromPDFViewToScreen(const WebCore::FloatRect&) const;
     WebCore::IntPoint convertFromRootViewToPDFView(const WebCore::IntPoint&) const;
     WebCore::IntRect boundsOnScreen() const;
-    
+
     bool showContextMenuAtPoint(const WebCore::IntPoint&);
 
-    String lookupTextAtLocation(const WebCore::FloatPoint&, WebHitTestResultData&, PDFSelection **, NSDictionary **) const;
+    std::tuple<String, RetainPtr<PDFSelection>, RetainPtr<NSDictionary>> lookupTextAtLocation(const WebCore::FloatPoint&, WebHitTestResultData&) const;
     WebCore::FloatRect rectForSelectionInRootView(PDFSelection *) const;
 
     CGFloat scaleFactor() const;
 
-    bool shouldPlaceBlockDirectionScrollbarOnLeft() const override { return false; }
-
     PDFPluginAnnotation* activeAnnotation() const { return m_activeAnnotation.get(); }
     WebCore::AXObjectCache* axObjectCache() const;
-    
+
 private:
-    explicit PDFPlugin(WebFrame*);
+    explicit PDFPlugin(WebFrame&);
 
     // Plugin functions.
-    bool initialize(const Parameters&) override;
-    void destroy() override;
-    void paint(WebCore::GraphicsContext&, const WebCore::IntRect& dirtyRectInWindowCoordinates) override { }
-    void updateControlTints(WebCore::GraphicsContext&) override;
-    bool supportsSnapshotting() const override { return true; }
-    RefPtr<ShareableBitmap> snapshot() override;
-    PlatformLayer* pluginLayer() override;
-    bool isTransparent() override { return false; }
-    bool wantsWheelEvents() override { return true; }
-    void geometryDidChange(const WebCore::IntSize& pluginSize, const WebCore::IntRect& clipRect, const WebCore::AffineTransform& pluginToRootViewTransform) override;
-    void contentsScaleFactorChanged(float) override;
-    void visibilityDidChange(bool) override { }
-    void frameDidFinishLoading(uint64_t requestID) override;
-    void frameDidFail(uint64_t requestID, bool wasCancelled) override;
-    void didEvaluateJavaScript(uint64_t requestID, const String& result) override;
-    void streamWillSendRequest(uint64_t streamID, const WebCore::URL& requestURL, const WebCore::URL& responseURL, int responseStatus) override { }
-    void streamDidReceiveResponse(uint64_t streamID, const WebCore::URL& responseURL, uint32_t streamLength, uint32_t lastModifiedTime, const String& mimeType, const String& headers, const String& suggestedFileName) override;
-    void streamDidReceiveData(uint64_t streamID, const char* bytes, int length) override;
-    void streamDidFinishLoading(uint64_t streamID) override;
-    void streamDidFail(uint64_t streamID, bool wasCancelled) override;
-    void manualStreamDidReceiveResponse(const WebCore::URL& responseURL, uint32_t streamLength, uint32_t lastModifiedTime, const WTF::String& mimeType, const WTF::String& headers, const String& suggestedFileName) override;
-    void manualStreamDidReceiveData(const char* bytes, int length) override;
-    void manualStreamDidFinishLoading() override;
-    void manualStreamDidFail(bool wasCancelled) override;
-    bool handleMouseEvent(const WebMouseEvent&) override;
-    bool handleWheelEvent(const WebWheelEvent&) override;
-    bool handleMouseEnterEvent(const WebMouseEvent&) override;
-    bool handleMouseLeaveEvent(const WebMouseEvent&) override;
-    bool handleContextMenuEvent(const WebMouseEvent&) override;
-    bool handleKeyboardEvent(const WebKeyboardEvent&) override;
-    bool handleEditingCommand(const String& commandName, const String& argument) override;
-    bool isEditingCommandEnabled(const String&) override;
-    bool handlesPageScaleFactor() const override;
-    bool requiresUnifiedScaleFactor() const override { return true; }
-    void setFocus(bool) override { }
-    NPObject* pluginScriptableNPObject() override { return 0; }
-    void windowFocusChanged(bool) override { }
-    void windowAndViewFramesChanged(const WebCore::IntRect& windowFrameInScreenCoordinates, const WebCore::IntRect& viewFrameInWindowCoordinates) override { }
-    void windowVisibilityChanged(bool) override { }
-    uint64_t pluginComplexTextInputIdentifier() const override { return 0; }
-    void sendComplexTextInput(const String& textInput) override { }
-    void setLayerHostingMode(LayerHostingMode) override { }
-    WebCore::Scrollbar* horizontalScrollbar() override { return m_horizontalScrollbar.get(); }
-    WebCore::Scrollbar* verticalScrollbar() override { return m_verticalScrollbar.get(); }
-    void storageBlockingStateChanged(bool) override { }
-    void privateBrowsingStateChanged(bool) override { }
-    bool getFormValue(String& formValue) override { return false; }
-    bool handleScroll(WebCore::ScrollDirection, WebCore::ScrollGranularity) override;
-    RefPtr<WebCore::SharedBuffer> liveResourceData() const override;
-    void willDetatchRenderer() override;
-    bool pluginHandlesContentOffsetForAccessibilityHitTest() const override;
+    bool initialize(const Parameters&) final;
+    void destroy() final;
+    void paint(WebCore::GraphicsContext&, const WebCore::IntRect& dirtyRectInWindowCoordinates) final { }
+    void updateControlTints(WebCore::GraphicsContext&) final;
+    bool supportsSnapshotting() const final { return true; }
+    RefPtr<ShareableBitmap> snapshot() final;
+    PlatformLayer* pluginLayer() final;
+    bool isTransparent() final { return false; }
+    bool wantsWheelEvents() final { return true; }
+    void geometryDidChange(const WebCore::IntSize& pluginSize, const WebCore::IntRect& clipRect, const WebCore::AffineTransform& pluginToRootViewTransform) final;
+    void contentsScaleFactorChanged(float) final;
+    void visibilityDidChange(bool) final { }
+    void frameDidFinishLoading(uint64_t requestID) final;
+    void frameDidFail(uint64_t requestID, bool wasCancelled) final;
+    void didEvaluateJavaScript(uint64_t requestID, const String& result) final;
+    void streamWillSendRequest(uint64_t streamID, const WebCore::URL& requestURL, const WebCore::URL& responseURL, int responseStatus) final { }
+    void streamDidReceiveResponse(uint64_t streamID, const WebCore::URL& responseURL, uint32_t streamLength, uint32_t lastModifiedTime, const String& mimeType, const String& headers, const String& suggestedFileName) final;
+    void streamDidReceiveData(uint64_t streamID, const char* bytes, int length) final;
+    void streamDidFinishLoading(uint64_t streamID) final;
+    void streamDidFail(uint64_t streamID, bool wasCancelled) final;
+    void manualStreamDidReceiveResponse(const WebCore::URL& responseURL, uint32_t streamLength, uint32_t lastModifiedTime, const WTF::String& mimeType, const WTF::String& headers, const String& suggestedFileName) final;
+    void manualStreamDidReceiveData(const char* bytes, int length) final;
+    void manualStreamDidFinishLoading() final;
+    void manualStreamDidFail(bool wasCancelled) final;
+    bool handleMouseEvent(const WebMouseEvent&) final;
+    bool handleWheelEvent(const WebWheelEvent&) final;
+    bool handleMouseEnterEvent(const WebMouseEvent&) final;
+    bool handleMouseLeaveEvent(const WebMouseEvent&) final;
+    bool handleContextMenuEvent(const WebMouseEvent&) final;
+    bool handleKeyboardEvent(const WebKeyboardEvent&) final;
+    bool handleEditingCommand(const String& commandName, const String& argument) final;
+    bool isEditingCommandEnabled(const String&) final;
+    bool handlesPageScaleFactor() const final;
+    bool requiresUnifiedScaleFactor() const final { return true; }
+    void setFocus(bool) final { }
+    NPObject* pluginScriptableNPObject() final { return nullptr; }
+    void windowFocusChanged(bool) final { }
+    void windowAndViewFramesChanged(const WebCore::IntRect& windowFrameInScreenCoordinates, const WebCore::IntRect& viewFrameInWindowCoordinates) final { }
+    void windowVisibilityChanged(bool) final { }
+    uint64_t pluginComplexTextInputIdentifier() const final { return 0; }
+    void sendComplexTextInput(const String& textInput) final { }
+    void setLayerHostingMode(LayerHostingMode) final { }
+    WebCore::Scrollbar* horizontalScrollbar() final { return m_horizontalScrollbar.get(); }
+    WebCore::Scrollbar* verticalScrollbar() final { return m_verticalScrollbar.get(); }
+    void storageBlockingStateChanged(bool) final { }
+    void privateBrowsingStateChanged(bool) final { }
+    bool getFormValue(String& formValue) final { return false; }
+    bool handleScroll(WebCore::ScrollDirection, WebCore::ScrollGranularity) final;
+    RefPtr<WebCore::SharedBuffer> liveResourceData() const final;
+    void willDetatchRenderer() final;
+    bool pluginHandlesContentOffsetForAccessibilityHitTest() const final;
     
-    bool isBeingAsynchronouslyInitialized() const override { return false; }
+    bool isBeingAsynchronouslyInitialized() const final { return false; }
 
-    RetainPtr<PDFDocument> pdfDocumentForPrinting() const override { return m_pdfDocument; }
-    NSObject *accessibilityObject() const override;
-    id accessibilityAssociatedPluginParentForElement(WebCore::Element*) const override;
+    RetainPtr<PDFDocument> pdfDocumentForPrinting() const final { return m_pdfDocument; }
+    NSObject *accessibilityObject() const final;
+    id accessibilityAssociatedPluginParentForElement(WebCore::Element*) const final;
 
-    unsigned countFindMatches(const String& target, WebCore::FindOptions, unsigned maxMatchCount) override;
-    bool findString(const String& target, WebCore::FindOptions, unsigned maxMatchCount) override;
+    unsigned countFindMatches(const String& target, WebCore::FindOptions, unsigned maxMatchCount) final;
+    bool findString(const String& target, WebCore::FindOptions, unsigned maxMatchCount) final;
 
     PDFSelection *nextMatchForString(const String& target, BOOL searchForward, BOOL caseSensitive, BOOL wrapSearch, PDFSelection *initialSelection, BOOL startInSelection);
 
-    bool performDictionaryLookupAtLocation(const WebCore::FloatPoint&) override;
-    String getSelectionString() const override;
-    String getSelectionForWordAtPoint(const WebCore::FloatPoint&) const override;
-    bool existingSelectionContainsPoint(const WebCore::FloatPoint&) const override;
+    bool performDictionaryLookupAtLocation(const WebCore::FloatPoint&) final;
+    String getSelectionString() const final;
+    String getSelectionForWordAtPoint(const WebCore::FloatPoint&) const final;
+    bool existingSelectionContainsPoint(const WebCore::FloatPoint&) const final;
 
-    bool shouldAllowScripting() override { return false; }
-    bool shouldAllowNavigationFromDrags() override { return true; }
-    bool shouldAlwaysAutoStart() const override { return true; }
+    bool shouldAllowScripting() final { return false; }
+    bool shouldAllowNavigationFromDrags() final { return true; }
+    bool shouldAlwaysAutoStart() const final { return true; }
 
     // ScrollableArea functions.
-    WebCore::IntRect scrollCornerRect() const override;
-    WebCore::ScrollableArea* enclosingScrollableArea() const override;
-    bool isScrollableOrRubberbandable() override { return true; }
-    bool hasScrollableOrRubberbandableAncestor() override { return true; }
-    WebCore::IntRect scrollableAreaBoundingBox(bool* = nullptr) const override;
-    void setScrollOffset(const WebCore::ScrollOffset&) override;
-    void invalidateScrollbarRect(WebCore::Scrollbar&, const WebCore::IntRect&) override;
-    void invalidateScrollCornerRect(const WebCore::IntRect&) override;
-    WebCore::IntPoint lastKnownMousePosition() const override { return m_lastMousePositionInPluginCoordinates; }
-    int scrollSize(WebCore::ScrollbarOrientation) const override;
-    bool isActive() const override;
-    bool isScrollCornerVisible() const override { return false; }
-    int scrollOffset(WebCore::ScrollbarOrientation) const override;
-    WebCore::ScrollPosition scrollPosition() const override;
-    WebCore::ScrollPosition minimumScrollPosition() const override;
-    WebCore::ScrollPosition maximumScrollPosition() const override;
-    WebCore::IntSize visibleSize() const override { return m_size; }
-    WebCore::IntSize contentsSize() const override { return m_pdfDocumentSize; }
-    WebCore::Scrollbar* horizontalScrollbar() const override { return m_horizontalScrollbar.get(); }
-    WebCore::Scrollbar* verticalScrollbar() const override { return m_verticalScrollbar.get(); }
-    bool shouldSuspendScrollAnimations() const override { return false; } // If we return true, ScrollAnimatorMac will keep cycling a timer forever, waiting for a good time to animate.
-    void scrollbarStyleChanged(WebCore::ScrollbarStyle, bool forceUpdate) override;
-    WebCore::IntRect convertFromScrollbarToContainingView(const WebCore::Scrollbar&, const WebCore::IntRect& scrollbarRect) const override;
-    WebCore::IntRect convertFromContainingViewToScrollbar(const WebCore::Scrollbar&, const WebCore::IntRect& parentRect) const override;
-    WebCore::IntPoint convertFromScrollbarToContainingView(const WebCore::Scrollbar&, const WebCore::IntPoint& scrollbarPoint) const override;
-    WebCore::IntPoint convertFromContainingViewToScrollbar(const WebCore::Scrollbar&, const WebCore::IntPoint& parentPoint) const override;
-    bool forceUpdateScrollbarsOnMainThreadForPerformanceTesting() const override;
+    WebCore::IntRect scrollCornerRect() const final;
+    WebCore::ScrollableArea* enclosingScrollableArea() const final;
+    bool isScrollableOrRubberbandable() final { return true; }
+    bool hasScrollableOrRubberbandableAncestor() final { return true; }
+    WebCore::IntRect scrollableAreaBoundingBox(bool* = nullptr) const final;
+    void setScrollOffset(const WebCore::ScrollOffset&) final;
+    void invalidateScrollbarRect(WebCore::Scrollbar&, const WebCore::IntRect&) final;
+    void invalidateScrollCornerRect(const WebCore::IntRect&) final;
+    WebCore::IntPoint lastKnownMousePosition() const final { return m_lastMousePositionInPluginCoordinates; }
+    int scrollSize(WebCore::ScrollbarOrientation) const final;
+    bool isActive() const final;
+    bool isScrollCornerVisible() const final { return false; }
+    int scrollOffset(WebCore::ScrollbarOrientation) const final;
+    WebCore::ScrollPosition scrollPosition() const final;
+    WebCore::ScrollPosition minimumScrollPosition() const final;
+    WebCore::ScrollPosition maximumScrollPosition() const final;
+    WebCore::IntSize visibleSize() const final { return m_size; }
+    WebCore::IntSize contentsSize() const final { return m_pdfDocumentSize; }
+    WebCore::Scrollbar* horizontalScrollbar() const final { return m_horizontalScrollbar.get(); }
+    WebCore::Scrollbar* verticalScrollbar() const final { return m_verticalScrollbar.get(); }
+    bool shouldSuspendScrollAnimations() const final { return false; } // If we return true, ScrollAnimatorMac will keep cycling a timer forever, waiting for a good time to animate.
+    void scrollbarStyleChanged(WebCore::ScrollbarStyle, bool forceUpdate) final;
+    WebCore::IntRect convertFromScrollbarToContainingView(const WebCore::Scrollbar&, const WebCore::IntRect& scrollbarRect) const final;
+    WebCore::IntRect convertFromContainingViewToScrollbar(const WebCore::Scrollbar&, const WebCore::IntRect& parentRect) const final;
+    WebCore::IntPoint convertFromScrollbarToContainingView(const WebCore::Scrollbar&, const WebCore::IntPoint& scrollbarPoint) const final;
+    WebCore::IntPoint convertFromContainingViewToScrollbar(const WebCore::Scrollbar&, const WebCore::IntPoint& parentPoint) const final;
+    bool forceUpdateScrollbarsOnMainThreadForPerformanceTesting() const final;
+    bool shouldPlaceBlockDirectionScrollbarOnLeft() const final { return false; }
 
     // PDFPlugin functions.
     void updateScrollbars();
@@ -258,16 +255,7 @@ private:
     WebFrame* webFrame() const { return m_frame; }
 
 #if __MAC_OS_X_VERSION_MIN_REQUIRED < 101300
-    enum UpdateCursorMode {
-        UpdateIfNeeded,
-        ForceUpdate
-    };
-
-    enum HitTestResult {
-        None,
-        Text
-    };
-
+    enum UpdateCursorMode { UpdateIfNeeded, ForceUpdate };
     void updateCursor(const WebMouseEvent&, UpdateCursorMode = UpdateIfNeeded);
 #endif
 
@@ -282,8 +270,8 @@ private:
 
     WebFrame* m_frame;
 
-    bool m_isPostScript;
-    bool m_pdfDocumentWasMutated;
+    bool m_isPostScript { false };
+    bool m_pdfDocumentWasMutated { false };
 
     WebCore::IntSize m_scrollOffset;
 
@@ -308,7 +296,8 @@ private:
     String m_lastFoundString;
 
 #if __MAC_OS_X_VERSION_MIN_REQUIRED < 101300
-    HitTestResult m_lastHitTestResult;
+    enum HitTestResult { None, Text };
+    HitTestResult m_lastHitTestResult { None };
 #endif
 
     RetainPtr<WKPDFLayerControllerDelegate> m_pdfLayerControllerDelegate;
@@ -333,5 +322,3 @@ private:
 SPECIALIZE_TYPE_TRAITS_PLUGIN(PDFPlugin, PDFPluginType)
 
 #endif // ENABLE(PDFKIT_PLUGIN)
-
-#endif // PDFPlugin_h
index 392f594..e3f88b1 100644 (file)
@@ -596,16 +596,14 @@ namespace WebKit {
 
 using namespace HTMLNames;
 
-Ref<PDFPlugin> PDFPlugin::create(WebFrame* frame)
+Ref<PDFPlugin> PDFPlugin::create(WebFrame& frame)
 {
     return adoptRef(*new PDFPlugin(frame));
 }
 
-PDFPlugin::PDFPlugin(WebFrame* frame)
+inline PDFPlugin::PDFPlugin(WebFrame& frame)
     : Plugin(PDFPluginType)
-    , m_frame(frame)
-    , m_isPostScript(false)
-    , m_pdfDocumentWasMutated(false)
+    , m_frame(&frame)
     , m_containerLayer(adoptNS([[CALayer alloc] init]))
     , m_contentLayer(adoptNS([[CALayer alloc] init]))
     , m_scrollCornerLayer(adoptNS([[WKPDFPluginScrollbarLayer alloc] initWithPDFPlugin:this]))
@@ -1848,13 +1846,14 @@ void PDFPlugin::showDefinitionForAttributedString(NSAttributedString *string, CG
     webFrame()->page()->send(Messages::WebPageProxy::DidPerformDictionaryLookup(dictionaryPopupInfo));
 }
 
-unsigned PDFPlugin::countFindMatches(const String& target, WebCore::FindOptions options, unsigned maxMatchCount)
+unsigned PDFPlugin::countFindMatches(const String& target, WebCore::FindOptions options, unsigned /*maxMatchCount*/)
 {
+    // FIXME: Why is it OK to ignore the passed-in maximum match count?
+
     if (!target.length())
         return 0;
 
-    int nsOptions = (options & FindOptionsCaseInsensitive) ? NSCaseInsensitiveSearch : 0;
-
+    NSStringCompareOptions nsOptions = options.contains(WebCore::CaseInsensitive) ? NSCaseInsensitiveSearch : 0;
     return [[pdfDocument() findString:target withOptions:nsOptions] count];
 }
 
@@ -1866,7 +1865,6 @@ PDFSelection *PDFPlugin::nextMatchForString(const String& target, BOOL searchFor
     NSStringCompareOptions options = 0;
     if (!searchForward)
         options |= NSBackwardsSearch;
-
     if (!caseSensitive)
         options |= NSCaseInsensitiveSearch;
 
@@ -1903,22 +1901,17 @@ PDFSelection *PDFPlugin::nextMatchForString(const String& target, BOOL searchFor
 
 bool PDFPlugin::findString(const String& target, WebCore::FindOptions options, unsigned maxMatchCount)
 {
-    BOOL searchForward = !(options & FindOptionsBackwards);
-    BOOL caseSensitive = !(options & FindOptionsCaseInsensitive);
-    BOOL wrapSearch = options & FindOptionsWrapAround;
+    bool searchForward = !options.contains(WebCore::Backwards);
+    bool caseSensitive = !options.contains(WebCore::CaseInsensitive);
+    bool wrapSearch = options.contains(WebCore::WrapAround);
 
-    unsigned matchCount;
-    if (!maxMatchCount) {
-        // If the max was zero, any result means we exceeded the max. We can skip computing the actual count.
-        matchCount = static_cast<unsigned>(kWKMoreThanMaximumMatchCount);
-    } else {
-        matchCount = countFindMatches(target, options, maxMatchCount);
-        if (matchCount > maxMatchCount)
-            matchCount = static_cast<unsigned>(kWKMoreThanMaximumMatchCount);
-    }
+    // If the max was zero, any result means we exceeded the max, so we can skip computing the actual count.
+    // FIXME: How can always returning true without searching if passed a max of 0 be right?
+    // Even if it is right, why not put that special case inside countFindMatches instead of here?
+    bool foundMatch = !maxMatchCount || countFindMatches(target, options, maxMatchCount);
 
     if (target.isEmpty()) {
-        PDFSelection* searchSelection = [m_pdfLayerController searchSelection];
+        auto searchSelection = [m_pdfLayerController searchSelection];
         [m_pdfLayerController findString:target caseSensitive:caseSensitive highlightMatches:YES];
         [m_pdfLayerController setSearchSelection:searchSelection];
         m_lastFoundString = emptyString();
@@ -1926,10 +1919,9 @@ bool PDFPlugin::findString(const String& target, WebCore::FindOptions options, u
     }
 
     if (m_lastFoundString == target) {
-        PDFSelection *selection = nextMatchForString(target, searchForward, caseSensitive, wrapSearch, [m_pdfLayerController searchSelection], NO);
+        auto selection = nextMatchForString(target, searchForward, caseSensitive, wrapSearch, [m_pdfLayerController searchSelection], NO);
         if (!selection)
             return false;
-
         [m_pdfLayerController setSearchSelection:selection];
         [m_pdfLayerController gotoSelection:selection];
     } else {
@@ -1937,17 +1929,15 @@ bool PDFPlugin::findString(const String& target, WebCore::FindOptions options, u
         m_lastFoundString = target;
     }
 
-    return matchCount > 0;
+    return foundMatch;
 }
 
 bool PDFPlugin::performDictionaryLookupAtLocation(const WebCore::FloatPoint& point)
 {
     IntPoint localPoint = convertFromRootViewToPlugin(roundedIntPoint(point));
     PDFSelection* lookupSelection = [m_pdfLayerController getSelectionForWordAtPoint:convertFromPluginToPDFView(localPoint)];
-
     if ([[lookupSelection string] length])
         [m_pdfLayerController searchInDictionaryWithSelection:lookupSelection];
-
     return true;
 }
 
@@ -1966,7 +1956,7 @@ void PDFPlugin::notifySelectionChanged(PDFSelection *)
     webFrame()->page()->didChangeSelection();
 }
 
-static const Cursor& pdfLayerControllerCursorTypeToCursor(PDFLayerControllerCursorType type)
+static const Cursor& coreCursor(PDFLayerControllerCursorType type)
 {
     switch (type) {
     case kPDFLayerControllerCursorTypeHand:
@@ -1981,7 +1971,7 @@ static const Cursor& pdfLayerControllerCursorTypeToCursor(PDFLayerControllerCurs
 
 void PDFPlugin::notifyCursorChanged(uint64_t type)
 {
-    webFrame()->page()->send(Messages::WebPageProxy::SetCursor(pdfLayerControllerCursorTypeToCursor(static_cast<PDFLayerControllerCursorType>(type))));
+    webFrame()->page()->send(Messages::WebPageProxy::SetCursor(coreCursor(static_cast<PDFLayerControllerCursorType>(type))));
 }
 
 String PDFPlugin::getSelectionString() const
@@ -1994,7 +1984,6 @@ String PDFPlugin::getSelectionForWordAtPoint(const WebCore::FloatPoint& point) c
     IntPoint pointInView = convertFromPluginToPDFView(convertFromRootViewToPlugin(roundedIntPoint(point)));
     PDFSelection *selectionForWord = [m_pdfLayerController getSelectionForWordAtPoint:pointInView];
     [m_pdfLayerController setCurrentSelection:selectionForWord];
-    
     return [selectionForWord string];
 }
 
@@ -2035,36 +2024,32 @@ static NSPoint pointInLayoutSpaceForPointInWindowSpace(PDFLayerController* pdfLa
     CGFloat scaleFactor = [pdfLayerController contentScaleFactor];
 
     scrollOffset.y = [pdfLayerController contentSizeRespectingZoom].height - NSRectToCGRect([pdfLayerController frame]).size.height - scrollOffset.y;
-    
+
     CGPoint newPoint = CGPointMake(scrollOffset.x + point.x, scrollOffset.y + point.y);
     newPoint.x /= scaleFactor;
     newPoint.y /= scaleFactor;
     return NSPointFromCGPoint(newPoint);
 }
 
-String PDFPlugin::lookupTextAtLocation(const WebCore::FloatPoint& locationInViewCoordinates, WebHitTestResultData& data, PDFSelection **selectionPtr, NSDictionary **options) const
+std::tuple<String, RetainPtr<PDFSelection>, RetainPtr<NSDictionary>> PDFPlugin::lookupTextAtLocation(const WebCore::FloatPoint& locationInViewCoordinates, WebHitTestResultData& data) const
 {
-    PDFSelection*& selection = *selectionPtr;
-
-    selection = [m_pdfLayerController currentSelection];
+    auto selection = [m_pdfLayerController currentSelection];
     if (existingSelectionContainsPoint(locationInViewCoordinates))
-        return selection.string;
-        
+        return { selection.string, selection, nil };
+
     IntPoint pointInPDFView = convertFromPluginToPDFView(convertFromRootViewToPlugin(roundedIntPoint(locationInViewCoordinates)));
     selection = [m_pdfLayerController getSelectionForWordAtPoint:pointInPDFView];
     if (!selection)
-        return "";
+        return { emptyString(), nil, nil };
 
     NSPoint pointInLayoutSpace = pointInLayoutSpaceForPointInWindowSpace(m_pdfLayerController.get(), pointInPDFView);
-
     PDFPage *currentPage = [[m_pdfLayerController layout] pageNearestPoint:pointInLayoutSpace currentPage:[m_pdfLayerController currentPage]];
-    
     NSPoint pointInPageSpace = [[m_pdfLayerController layout] convertPoint:pointInLayoutSpace toPage:currentPage forScaleFactor:1.0];
-    
+
     for (PDFAnnotation *annotation in currentPage.annotations) {
         if (![annotation isKindOfClass:pdfAnnotationLinkClass()])
             continue;
-    
+
         NSRect bounds = annotation.bounds;
         if (!NSPointInRect(pointInPageSpace, bounds))
             continue;
@@ -2076,18 +2061,19 @@ String PDFPlugin::lookupTextAtLocation(const WebCore::FloatPoint& locationInView
         NSURL *url = linkAnnotation.URL;
         if (!url)
             continue;
-        
+
         data.absoluteLinkURL = url.absoluteString;
         data.linkLabel = selection.string;
-        return selection.string;
+        return { selection.string, selection, nil };
     }
-    
-    NSString *lookupText = DictionaryLookup::stringForPDFSelection(selection, options);
-    if (!lookupText || !lookupText.length)
-        return "";
+
+    RetainPtr<NSDictionary> options;
+    NSString *lookupText = DictionaryLookup::stringForPDFSelection(selection, &options);
+    if (!lookupText.length)
+        return { emptyString(), selection, nil };
 
     [m_pdfLayerController setCurrentSelection:selection];
-    return lookupText;
+    return { lookupText, selection, WTFMove(options) };
 }
 
 static NSRect rectInViewSpaceForRectInLayoutSpace(PDFLayerController* pdfLayerController, NSRect layoutSpaceRect)
@@ -2117,20 +2103,20 @@ WebCore::AXObjectCache* PDFPlugin::axObjectCache() const
 WebCore::FloatRect PDFPlugin::rectForSelectionInRootView(PDFSelection *selection) const
 {
     PDFPage *currentPage = nil;
-    NSArraypages = selection.pages;
+    NSArray *pages = selection.pages;
     if (pages.count)
         currentPage = (PDFPage *)[pages objectAtIndex:0];
 
     if (!currentPage)
         currentPage = [m_pdfLayerController currentPage];
-    
+
     NSRect rectInPageSpace = [selection boundsForPage:currentPage];
     NSRect rectInLayoutSpace = [[m_pdfLayerController layout] convertRect:rectInPageSpace fromPage:currentPage forScaleFactor:1.0];
     NSRect rectInView = rectInViewSpaceForRectInLayoutSpace(m_pdfLayerController.get(), rectInLayoutSpace);
 
     rectInView.origin = convertFromPDFViewToRootView(IntPoint(rectInView.origin));
 
-    return WebCore::FloatRect(rectInView);
+    return rectInView;
 }
 
 CGFloat PDFPlugin::scaleFactor() const
@@ -2198,7 +2184,7 @@ NSData *PDFPlugin::liveData() const
     // untouched by the user, so that PDFs which PDFKit can't display will still be downloadable.
     if (m_pdfDocumentWasMutated)
         return [m_pdfDocument dataRepresentation];
-    
+
     return rawData();
 }
 
@@ -2207,16 +2193,16 @@ id PDFPlugin::accessibilityAssociatedPluginParentForElement(WebCore::Element* el
 #if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101300
     if (!m_activeAnnotation)
         return nil;
-    
+
     if (m_activeAnnotation->element() != element)
         return nil;
-    
+
     return [m_activeAnnotation->annotation() accessibilityNode];
 #else
     return nil;
 #endif
 }
-    
+
 NSObject *PDFPlugin::accessibilityObject() const
 {
     return m_accessibilityObject.get();
index 8e228c8..dc4211d 100644 (file)
@@ -23,8 +23,7 @@
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef PluginProxy_h
-#define PluginProxy_h
+#pragma once
 
 #if ENABLE(NETSCAPE_PLUGIN_API)
 
@@ -32,7 +31,6 @@
 #include "Plugin.h"
 #include "PluginProcess.h"
 #include <WebCore/AffineTransform.h>
-#include <WebCore/FindOptions.h>
 #include <WebCore/IntRect.h>
 #include <WebCore/SecurityOrigin.h>
 #include <memory>
@@ -238,4 +236,3 @@ SPECIALIZE_TYPE_TRAITS_PLUGIN(PluginProxy, PluginProxyType)
 
 #endif // ENABLE(NETSCAPE_PLUGIN_API)
 
-#endif // PluginProxy_h
index 186bfd7..33e6712 100644 (file)
@@ -1768,20 +1768,8 @@ bool WebFrameLoaderClient::allowScript(bool enabledPerSettings)
     if (!enabledPerSettings)
         return false;
 
-    Frame* coreFrame = m_frame->coreFrame();
-
-    if (coreFrame->document()->isPluginDocument()) {
-        PluginDocument* pluginDocument = static_cast<PluginDocument*>(coreFrame->document());
-
-        if (pluginDocument->pluginWidget() && pluginDocument->pluginWidget()->isPluginView()) {
-            PluginView* pluginView = static_cast<PluginView*>(pluginDocument->pluginWidget());
-
-            if (!pluginView->shouldAllowScripting())
-                return false;
-        }
-    }
-
-    return true;
+    auto* pluginView = WebPage::pluginViewForFrame(m_frame->coreFrame());
+    return !pluginView || !pluginView->shouldAllowScripting();
 }
 
 bool WebFrameLoaderClient::shouldForceUniversalAccessFromLocalURL(const WebCore::URL& url)
index 026cd07..7525aed 100644 (file)
@@ -45,7 +45,7 @@ namespace WebKit {
 
 void WebContextMenuClient::lookUpInDictionary(Frame* frame)
 {
-    m_page->performDictionaryLookupForSelection(frame, frame->selection().selection(), TextIndicatorPresentationTransition::BounceAndCrossfade);
+    m_page->performDictionaryLookupForSelection(*frame, frame->selection().selection(), TextIndicatorPresentationTransition::BounceAndCrossfade);
 }
 
 bool WebContextMenuClient::isSpeaking()
@@ -67,7 +67,7 @@ void WebContextMenuClient::searchWithGoogle(const Frame* frame)
 {
     String searchString = frame->editor().selectedText();
     searchString.stripWhiteSpace();
-    
+
     m_page->send(Messages::WebPageProxy::SearchTheWeb(searchString));
 }
 
@@ -78,7 +78,7 @@ void WebContextMenuClient::searchWithSpotlight()
     // If not, can we find a place in WebCore to put this?
 
     Frame& mainFrame = m_page->corePage()->mainFrame();
-    
+
     Frame* selectionFrame = &mainFrame;
     for (; selectionFrame; selectionFrame = selectionFrame->tree().traverseNext()) {
         if (selectionFrame->selection().isRange())
@@ -88,7 +88,7 @@ void WebContextMenuClient::searchWithSpotlight()
         selectionFrame = &mainFrame;
 
     String selectedString = selectionFrame->displayStringModifiedByEncoding(selectionFrame->editor().selectedText());
-    
+
     if (selectedString.isEmpty())
         return;
 
index e56423a..e19962f 100644 (file)
@@ -53,20 +53,24 @@ using namespace WebCore;
 
 namespace WebKit {
 
-static WebCore::FindOptions core(FindOptions options)
+WebCore::FindOptions core(FindOptions options)
 {
-    return (options & FindOptionsCaseInsensitive ? CaseInsensitive : 0)
-        | (options & FindOptionsAtWordStarts ? AtWordStarts : 0)
-        | (options & FindOptionsTreatMedialCapitalAsWordStart ? TreatMedialCapitalAsWordStart : 0)
-        | (options & FindOptionsBackwards ? Backwards : 0)
-        | (options & FindOptionsWrapAround ? WrapAround : 0);
+    WebCore::FindOptions result;
+    if (options & FindOptionsCaseInsensitive)
+        result |= WebCore::CaseInsensitive;
+    if (options & FindOptionsAtWordStarts)
+        result |= WebCore::AtWordStarts;
+    if (options & FindOptionsTreatMedialCapitalAsWordStart)
+        result |= WebCore::TreatMedialCapitalAsWordStart;
+    if (options & FindOptionsBackwards)
+        result |= WebCore::Backwards;
+    if (options & FindOptionsWrapAround)
+        result |= WebCore::WrapAround;
+    return result;
 }
 
 FindController::FindController(WebPage* webPage)
     : m_webPage(webPage)
-    , m_findPageOverlay(nullptr)
-    , m_isShowingFindIndicator(false)
-    , m_foundStringMatchIndex(-1)
 {
 }
 
@@ -74,24 +78,14 @@ FindController::~FindController()
 {
 }
 
-static PluginView* pluginViewForFrame(Frame* frame)
-{
-    if (!frame->document()->isPluginDocument())
-        return 0;
-
-    PluginDocument* pluginDocument = static_cast<PluginDocument*>(frame->document());
-    return static_cast<PluginView*>(pluginDocument->pluginWidget());
-}
-
 void FindController::countStringMatches(const String& string, FindOptions options, unsigned maxMatchCount)
 {
     if (maxMatchCount == std::numeric_limits<unsigned>::max())
         --maxMatchCount;
     
-    PluginView* pluginView = pluginViewForFrame(m_webPage->mainFrame());
+    auto* pluginView = WebPage::pluginViewForFrame(m_webPage->mainFrame());
     
     unsigned matchCount;
-
     if (pluginView)
         matchCount = pluginView->countFindMatches(string, core(options), maxMatchCount + 1);
     else {
@@ -119,7 +113,7 @@ void FindController::updateFindUIAfterPageScroll(bool found, const String& strin
 {
     Frame* selectedFrame = frameWithSelection(m_webPage->corePage());
     
-    PluginView* pluginView = pluginViewForFrame(m_webPage->mainFrame());
+    auto* pluginView = WebPage::pluginViewForFrame(m_webPage->mainFrame());
 
     bool shouldShowOverlay = false;
 
@@ -170,7 +164,7 @@ void FindController::updateFindUIAfterPageScroll(bool found, const String& strin
             m_foundStringMatchIndex = -1;
         else {
             if (m_foundStringMatchIndex < 0)
-                m_foundStringMatchIndex += matchCount;
+                m_foundStringMatchIndex += matchCount; // FIXME: Shouldn't this just be "="? Why is it correct to add to -1 here?
             if (m_foundStringMatchIndex >= (int) matchCount)
                 m_foundStringMatchIndex -= matchCount;
         }
@@ -203,7 +197,7 @@ void FindController::updateFindUIAfterPageScroll(bool found, const String& strin
 
 void FindController::findString(const String& string, FindOptions options, unsigned maxMatchCount)
 {
-    PluginView* pluginView = pluginViewForFrame(m_webPage->mainFrame());
+    auto* pluginView = WebPage::pluginViewForFrame(m_webPage->mainFrame());
 
     WebCore::FindOptions coreOptions = core(options);
 
@@ -211,7 +205,7 @@ void FindController::findString(const String& string, FindOptions options, unsig
     // we need to avoid sending the non-painted selection change to the UI process
     // so that it does not clear the selection out from under us.
 #if PLATFORM(IOS)
-    coreOptions = static_cast<FindOptions>(coreOptions | DoNotRevealSelection);
+    coreOptions |= DoNotRevealSelection;
 #endif
 
     willFindString();
@@ -316,10 +310,8 @@ void FindController::hideFindUI()
     if (m_findPageOverlay)
         m_webPage->mainFrame()->pageOverlayController().uninstallPageOverlay(*m_findPageOverlay, PageOverlay::FadeMode::Fade);
 
-    PluginView* pluginView = pluginViewForFrame(m_webPage->mainFrame());
-    
-    if (pluginView)
-        pluginView->findString(emptyString(), 0, 0);
+    if (auto* pluginView = WebPage::pluginViewForFrame(m_webPage->mainFrame()))
+        pluginView->findString(emptyString(), { }, 0);
     else
         m_webPage->corePage()->unmarkAllTextMatches();
     
index 619e588..8a8d4a3 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef FindController_h
-#define FindController_h
+#pragma once
 
 #include "ShareableBitmap.h"
 #include "WebFindOptions.h"
+#include <WebCore/FindOptions.h>
 #include <WebCore/IntRect.h>
 #include <WebCore/PageOverlay.h>
 #include <wtf/Forward.h>
@@ -41,7 +41,6 @@
 namespace WebCore {
 class Frame;
 class Range;
-
 enum class DidWrap : bool;
 }
 
@@ -91,15 +90,15 @@ private:
     void didHideFindIndicator();
 
     WebPage* m_webPage;
-    WebCore::PageOverlay* m_findPageOverlay;
+    WebCore::PageOverlay* m_findPageOverlay { nullptr };
 
     // Whether the UI process is showing the find indicator. Note that this can be true even if
     // the find indicator isn't showing, but it will never be false when it is showing.
-    bool m_isShowingFindIndicator;
+    bool m_isShowingFindIndicator { false };
     WebCore::IntRect m_findIndicatorRect;
     Vector<RefPtr<WebCore::Range>> m_findMatches;
     // Index value is -1 if not found or if number of matches exceeds provided maximum.
-    int m_foundStringMatchIndex;
+    int m_foundStringMatchIndex { -1 };
 
 #if PLATFORM(IOS)
     RefPtr<WebCore::PageOverlay> m_findIndicatorOverlay;
@@ -107,6 +106,6 @@ private:
 #endif
 };
 
-} // namespace WebKit
+WebCore::FindOptions core(FindOptions);
 
-#endif // FindController_h
+} // namespace WebKit
index c288a03..20c1762 100644 (file)
@@ -518,21 +518,13 @@ JSGlobalContextRef WebFrame::jsContextForWorld(InjectedBundleScriptWorld* world)
 
 bool WebFrame::handlesPageScaleGesture() const
 {
-    if (!m_coreFrame->document()->isPluginDocument())
-        return 0;
-
-    PluginDocument* pluginDocument = static_cast<PluginDocument*>(m_coreFrame->document());
-    PluginView* pluginView = static_cast<PluginView*>(pluginDocument->pluginWidget());
+    auto* pluginView = WebPage::pluginViewForFrame(m_coreFrame);
     return pluginView && pluginView->handlesPageScaleFactor();
 }
 
 bool WebFrame::requiresUnifiedScaleFactor() const
 {
-    if (!m_coreFrame->document()->isPluginDocument())
-        return 0;
-
-    PluginDocument* pluginDocument = static_cast<PluginDocument*>(m_coreFrame->document());
-    PluginView* pluginView = static_cast<PluginView*>(pluginDocument->pluginWidget());
+    auto* pluginView = WebPage::pluginViewForFrame(m_coreFrame);
     return pluginView && pluginView->requiresUnifiedScaleFactor();
 }
 
index 2c4229c..6fe79b4 100644 (file)
@@ -801,12 +801,8 @@ RefPtr<Plugin> WebPage::createPlugin(WebFrame* frame, HTMLPlugInElement* pluginE
     if (isBlockedPlugin || !pluginProcessToken) {
 #if ENABLE(PDFKIT_PLUGIN)
         String path = parameters.url.path();
-        if (shouldUsePDFPlugin() && (MIMETypeRegistry::isPDFOrPostScriptMIMEType(parameters.mimeType) || (parameters.mimeType.isEmpty() && (path.endsWithIgnoringASCIICase(".pdf") || path.endsWithIgnoringASCIICase(".ps"))))) {
-            auto pdfPlugin = PDFPlugin::create(frame);
-            return WTFMove(pdfPlugin);
-        }
-#else
-        UNUSED_PARAM(frame);
+        if (shouldUsePDFPlugin() && (MIMETypeRegistry::isPDFOrPostScriptMIMEType(parameters.mimeType) || (parameters.mimeType.isEmpty() && (path.endsWithIgnoringASCIICase(".pdf") || path.endsWithIgnoringASCIICase(".ps")))))
+            return PDFPlugin::create(*frame);
 #endif
     }
 
@@ -1004,26 +1000,23 @@ Ref<API::Array> WebPage::trackedRepaintRects()
 
 PluginView* WebPage::focusedPluginViewForFrame(Frame& frame)
 {
-    if (!frame.document()->isPluginDocument())
-        return 0;
-
-    PluginDocument* pluginDocument = static_cast<PluginDocument*>(frame.document());
+    if (!is<PluginDocument>(frame.document()))
+        return nullptr;
 
-    if (pluginDocument->focusedElement() != pluginDocument->pluginElement())
-        return 0;
+    auto& pluginDocument = downcast<PluginDocument>(*frame.document());
+    if (pluginDocument.focusedElement() != pluginDocument.pluginElement())
+        return nullptr;
 
-    PluginView* pluginView = static_cast<PluginView*>(pluginDocument->pluginWidget());
-    return pluginView;
+    return pluginViewForFrame(&frame);
 }
 
 PluginView* WebPage::pluginViewForFrame(Frame* frame)
 {
-    if (!frame->document()->isPluginDocument())
-        return 0;
+    if (!frame || !is<PluginDocument>(frame->document()))
+        return nullptr;
 
-    PluginDocument* pluginDocument = static_cast<PluginDocument*>(frame->document());
-    PluginView* pluginView = static_cast<PluginView*>(pluginDocument->pluginWidget());
-    return pluginView;
+    auto& document = downcast<PluginDocument>(*frame->document());
+    return static_cast<PluginView*>(document.pluginWidget());
 }
 
 void WebPage::executeEditingCommand(const String& commandName, const String& argument)
@@ -3482,7 +3475,7 @@ void WebPage::setActiveOpenPanelResultListener(Ref<WebOpenPanelResultListener>&&
 
 bool WebPage::findStringFromInjectedBundle(const String& target, FindOptions options)
 {
-    return m_page->findString(target, options);
+    return m_page->findString(target, core(options));
 }
 
 void WebPage::findString(const String& string, uint32_t options, uint32_t maxMatchCount)
@@ -4063,19 +4056,17 @@ void WebPage::stopSpeaking()
 #endif
 
 #if PLATFORM(MAC)
+
 RetainPtr<PDFDocument> WebPage::pdfDocumentForPrintingFrame(Frame* coreFrame)
 {
-    Document* document = coreFrame->document();
-    if (!is<PluginDocument>(document))
-        return nullptr;
-
-    PluginView* pluginView = static_cast<PluginView*>(downcast<PluginDocument>(*document).pluginWidget());
+    PluginView* pluginView = pluginViewForFrame(coreFrame);
     if (!pluginView)
         return nullptr;
 
     return pluginView->pdfDocumentForPrinting();
 }
-#endif // PLATFORM(MAC)
+
+#endif
 
 void WebPage::beginPrinting(uint64_t frameID, const PrintInfo& printInfo)
 {
@@ -4090,7 +4081,7 @@ void WebPage::beginPrinting(uint64_t frameID, const PrintInfo& printInfo)
 #if PLATFORM(MAC)
     if (pdfDocumentForPrintingFrame(coreFrame))
         return;
-#endif // PLATFORM(MAC)
+#endif
 
     if (!m_printContext)
         m_printContext = std::make_unique<PrintContext>(coreFrame);
index 4ef6491..19b6825 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010-2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -115,31 +115,32 @@ class Connection;
 }
 
 namespace WebCore {
+
 class CaptureDevice;
 class DocumentLoader;
 class DragData;
-class GraphicsContext;
 class Frame;
 class FrameSelection;
 class FrameView;
+class GraphicsContext;
 class HTMLPlugInElement;
 class HTMLPlugInImageElement;
 class IntPoint;
 class KeyboardEvent;
 class MediaPlaybackTargetContext;
+class MediaPlayerRequestInstallMissingPluginsCallback;
 class Page;
 class PrintContext;
 class Range;
-class ResourceResponse;
 class ResourceRequest;
+class ResourceResponse;
 class SelectionRect;
 class SharedBuffer;
 class SubstituteData;
 class TextCheckingRequest;
 class URL;
 class VisiblePosition;
-enum class TextIndicatorPresentationTransition : uint8_t;
-enum SyntheticClickType : int8_t;
+
 struct CompositionUnderline;
 struct DictationAlternative;
 struct Highlight;
@@ -147,12 +148,14 @@ struct KeypressCommand;
 struct TextCheckingResult;
 struct ViewportArguments;
 
-#if ENABLE(VIDEO) && USE(GSTREAMER)
-class MediaPlayerRequestInstallMissingPluginsCallback;
-#endif
+enum SyntheticClickType : int8_t;
+
+enum class TextIndicatorPresentationTransition : uint8_t;
+
 }
 
 namespace WebKit {
+
 class DataReference;
 class DrawingArea;
 class DownloadID;
@@ -194,6 +197,9 @@ class WebUndoStep;
 class WebUserContentController;
 class VideoFullscreenManager;
 class WebWheelEvent;
+class WebTouchEvent;
+class RemoteLayerTreeTransaction;
+
 struct AssistedNodeInformation;
 struct AttributedString;
 struct BackForwardListItemState;
@@ -202,24 +208,16 @@ struct InteractionInformationAtPosition;
 struct InteractionInformationRequest;
 struct LoadParameters;
 struct PrintInfo;
-struct WebsitePolicies;
 struct WebPageCreationParameters;
 struct WebPreferencesStore;
 struct WebSelectionData;
+struct WebsitePolicies;
 
 enum class DragControllerAction;
 enum FindOptions : uint16_t;
 
-typedef uint32_t SnapshotOptions;
-typedef uint32_t WKEventModifiers;
-
-#if PLATFORM(COCOA)
-class RemoteLayerTreeTransaction;
-#endif
-
-#if ENABLE(TOUCH_EVENTS)
-class WebTouchEvent;
-#endif
+using SnapshotOptions = uint32_t;
+using WKEventModifiers = uint32_t;
 
 class WebPage : public API::ObjectImpl<API::Object::Type::BundlePage>, public IPC::MessageReceiver, public IPC::MessageSender {
 public:
@@ -245,6 +243,7 @@ public:
     
     InjectedBundleBackForwardList* backForwardList();
     DrawingArea* drawingArea() const { return m_drawingArea.get(); }
+
 #if ENABLE(ASYNC_SCROLLING)
     WebCore::ScrollingCoordinator* scrollingCoordinator() const;
 #endif
@@ -273,6 +272,7 @@ public:
     PlaybackSessionManager& playbackSessionManager();
     VideoFullscreenManager& videoFullscreenManager();
 #endif
+
 #if PLATFORM(IOS)
     void setAllowsMediaDocumentInlinePlayback(bool);
     bool allowsMediaDocumentInlinePlayback() const { return m_allowsMediaDocumentInlinePlayback; }
@@ -345,14 +345,14 @@ public:
 #endif
 
 #if ENABLE(CONTEXT_MENUS)
-    API::InjectedBundle::PageContextMenuClient& injectedBundleContextMenuClient() { return *m_contextMenuClient.get(); }
+    API::InjectedBundle::PageContextMenuClient& injectedBundleContextMenuClient() { return *m_contextMenuClient; }
 #endif
-    API::InjectedBundle::EditorClient& injectedBundleEditorClient() { return *m_editorClient.get(); }
-    API::InjectedBundle::FormClient& injectedBundleFormClient() { return *m_formClient.get(); }
+    API::InjectedBundle::EditorClient& injectedBundleEditorClient() { return *m_editorClient; }
+    API::InjectedBundle::FormClient& injectedBundleFormClient() { return *m_formClient; }
     API::InjectedBundle::PageLoaderClient& injectedBundleLoaderClient() { return *m_loaderClient; }
     InjectedBundlePagePolicyClient& injectedBundlePolicyClient() { return m_policyClient; }
     API::InjectedBundle::ResourceLoadClient& injectedBundleResourceLoadClient() { return *m_resourceLoadClient; }
-    API::InjectedBundle::PageUIClient& injectedBundleUIClient() { return *m_uiClient.get(); }
+    API::InjectedBundle::PageUIClient& injectedBundleUIClient() { return *m_uiClient; }
 #if ENABLE(FULLSCREEN_API)
     InjectedBundlePageFullScreenClient& injectedBundleFullScreenClient() { return m_fullScreenClient; }
 #endif
@@ -361,8 +361,8 @@ public:
 
     WebFrame* mainWebFrame() const { return m_mainFrame.get(); }
 
-    WebCore::MainFrame* mainFrame() const; // May return 0.
-    WebCore::FrameView* mainFrameView() const; // May return 0.
+    WebCore::MainFrame* mainFrame() const; // May return nullptr.
+    WebCore::FrameView* mainFrameView() const; // May return nullptr.
 
     RefPtr<WebCore::Range> currentSelectionAsRange();
 
@@ -588,9 +588,6 @@ public:
     void getRectsForGranularityWithSelectionOffset(uint32_t, int32_t, CallbackID);
     void getRectsAtSelectionOffsetWithText(int32_t, const String&, CallbackID);
     void storeSelectionForAccessibility(bool);
-#if ENABLE(IOS_TOUCH_EVENTS)
-    void dispatchAsynchronousTouchEvents(const Vector<WebTouchEvent, 1>& queue);
-#endif
 
     void contentSizeCategoryDidChange(const String&);
 
@@ -607,6 +604,11 @@ public:
     
     void setForceAlwaysUserScalable(bool);
 #endif
+
+#if PLATFORM(IOS) && ENABLE(IOS_TOUCH_EVENTS)
+    void dispatchAsynchronousTouchEvents(const Vector<WebTouchEvent, 1>& queue);
+#endif
+
     bool hasRichlyEditableSelection() const;
 
     void setLayerTreeStateIsFrozen(bool);
@@ -616,6 +618,7 @@ public:
     NotificationPermissionRequestManager* notificationPermissionRequestManager();
 
     void pageDidScroll();
+
 #if USE(COORDINATED_GRAPHICS)
     void pageDidRequestScroll(const WebCore::IntPoint&);
 #endif
@@ -624,7 +627,7 @@ public:
     WebContextMenu* contextMenu();
     WebContextMenu* contextMenuAtPointInWindow(const WebCore::IntPoint&);
 #endif
-    
+
     bool hasLocalDataForURL(const WebCore::URL&);
     String cachedResponseMIMETypeForURL(const WebCore::URL&);
     String cachedSuggestedFilenameForURL(const WebCore::URL&);
@@ -690,22 +693,22 @@ public:
     void setCompositionAsync(const String& text, const Vector<WebCore::CompositionUnderline>& underlines, const EditingRange& selectionRange, const EditingRange& replacementRange);
     void confirmCompositionAsync();
 
-#if PLATFORM(MAC)
-    void insertDictatedTextAsync(const String& text, const EditingRange& replacementRange, const Vector<WebCore::DictationAlternative>& dictationAlternativeLocations, bool registerUndoGroup = false);
-    void attributedSubstringForCharacterRangeAsync(const EditingRange&, CallbackID);
-    void fontAtSelection(CallbackID);
-#endif
-
     void readSelectionFromPasteboard(const WTF::String& pasteboardName, bool& result);
     void getStringSelectionForPasteboard(WTF::String& stringValue);
     void getDataSelectionForPasteboard(const WTF::String pasteboardType, SharedMemory::Handle& handle, uint64_t& size);
     void shouldDelayWindowOrderingEvent(const WebKit::WebMouseEvent&, bool& result);
     void acceptsFirstMouse(int eventNumber, const WebKit::WebMouseEvent&, bool& result);
     bool performNonEditingBehaviorForSelector(const String&, WebCore::KeyboardEvent*);
+#endif
 
-#if ENABLE(SERVICE_CONTROLS)
-    void replaceSelectionWithPasteboardData(const Vector<String>& types, const IPC::DataReference&);
+#if PLATFORM(MAC)
+    void insertDictatedTextAsync(const String& text, const EditingRange& replacementRange, const Vector<WebCore::DictationAlternative>& dictationAlternativeLocations, bool registerUndoGroup = false);
+    void attributedSubstringForCharacterRangeAsync(const EditingRange&, CallbackID);
+    void fontAtSelection(CallbackID);
 #endif
+
+#if PLATFORM(COCOA) && ENABLE(SERVICE_CONTROLS)
+    void replaceSelectionWithPasteboardData(const Vector<String>& types, const IPC::DataReference&);
 #endif
 
 #if HAVE(ACCESSIBILITY) && PLATFORM(GTK)
@@ -725,7 +728,7 @@ public:
     void speak(const String&);
     void stopSpeaking();
 
-    void performDictionaryLookupForSelection(WebCore::Frame*, const WebCore::VisibleSelection&, WebCore::TextIndicatorPresentationTransition);
+    void performDictionaryLookupForSelection(WebCore::Frame&, const WebCore::VisibleSelection&, WebCore::TextIndicatorPresentationTransition);
 #endif
 
     bool isSmartInsertDeleteEnabled();
@@ -738,12 +741,15 @@ public:
     void clearSelection();
     void restoreSelectionInFocusedEditableElement();
 
-#if ENABLE(DRAG_SUPPORT)
-#if PLATFORM(GTK)
+#if ENABLE(DRAG_SUPPORT) && PLATFORM(GTK)
     void performDragControllerAction(DragControllerAction, const WebCore::IntPoint& clientPosition, const WebCore::IntPoint& globalPosition, uint64_t draggingSourceOperationMask, WebSelectionData&&, uint32_t flags);
-#else
+#endif
+
+#if ENABLE(DRAG_SUPPORT) && !PLATFORM(GTK)
     void performDragControllerAction(DragControllerAction, const WebCore::DragData&, const SandboxExtension::Handle&, const SandboxExtension::HandleArray&);
 #endif
+
+#if ENABLE(DRAG_SUPPORT)
     void dragEnded(WebCore::IntPoint clientPosition, WebCore::IntPoint globalPosition, uint64_t operation);
 
     void willPerformLoadDragDestinationAction();
@@ -752,20 +758,24 @@ public:
     void willStartDrag() { ASSERT(!m_isStartingDrag); m_isStartingDrag = true; }
     void didStartDrag();
     void dragCancelled();
-#endif // ENABLE(DRAG_SUPPORT)
+#endif
 
     void beginPrinting(uint64_t frameID, const PrintInfo&);
     void endPrinting();
     void computePagesForPrinting(uint64_t frameID, const PrintInfo&, CallbackID);
     void computePagesForPrintingImpl(uint64_t frameID, const PrintInfo&, Vector<WebCore::IntRect>& pageRects, double& totalScaleFactor);
+
 #if PLATFORM(COCOA)
     void drawRectToImage(uint64_t frameID, const PrintInfo&, const WebCore::IntRect&, const WebCore::IntSize&, CallbackID);
     void drawPagesToPDF(uint64_t frameID, const PrintInfo&, uint32_t first, uint32_t count, CallbackID);
     void drawPagesToPDFImpl(uint64_t frameID, const PrintInfo&, uint32_t first, uint32_t count, RetainPtr<CFMutableDataRef>& pdfPageData);
+#endif
+
 #if PLATFORM(IOS)
     void computePagesForPrintingAndDrawToPDF(uint64_t frameID, const PrintInfo&, CallbackID, Ref<Messages::WebPage::ComputePagesForPrintingAndDrawToPDF::DelayedReply>&&);
 #endif
-#elif PLATFORM(GTK)
+
+#if PLATFORM(GTK)
     void drawPagesForPrinting(uint64_t frameID, const PrintInfo&, CallbackID);
     void didFinishPrintOperation(const WebCore::ResourceError&, CallbackID);
 #endif
@@ -805,6 +815,7 @@ public:
 
     void unmarkAllMisspellings();
     void unmarkAllBadGrammar();
+
 #if PLATFORM(COCOA)
     void handleAlternativeTextUIResult(const String&);
 #endif
@@ -878,6 +889,7 @@ public:
 #endif
 
     void savePDFToFileInDownloadsFolder(const String& suggestedFilename, const WebCore::URL& originatingURL, const uint8_t* data, unsigned long size);
+
 #if PLATFORM(COCOA)
     void savePDFToTemporaryFolderAndOpenWithNativeApplication(const String& suggestedFilename, const String& originatingURLString, const uint8_t* data, unsigned long size, const String& pdfUUID);
 #endif
@@ -946,11 +958,10 @@ public:
 #endif
 
     void imageOrMediaDocumentSizeChanged(const WebCore::IntSize&);
-#if ENABLE(VIDEO)
-#if USE(GSTREAMER)
+
+#if ENABLE(VIDEO) && USE(GSTREAMER)
     void requestInstallMissingMediaPlugins(const String& details, const String& description, WebCore::MediaPlayerRequestInstallMissingPluginsCallback&);
 #endif
-#endif
 
     void addUserScript(String&& source, WebCore::UserContentInjectedFrames, WebCore::UserScriptInjectionTime);
     void addUserStyleSheet(const String& source, WebCore::UserContentInjectedFrames);
@@ -1037,11 +1048,12 @@ private:
     void resetTextAutosizing();
     WebCore::VisiblePosition visiblePositionInFocusedNodeForPoint(const WebCore::Frame&, const WebCore::IntPoint&, bool isInteractingWithAssistedNode);
     RefPtr<WebCore::Range> rangeForGranularityAtPoint(WebCore::Frame&, const WebCore::IntPoint&, uint32_t granularity, bool isInteractingWithAssistedNode);
-#if ENABLE(DATA_INTERACTION)
+#endif
+
+#if PLATFORM(IOS) && ENABLE(DATA_INTERACTION)
     void requestStartDataInteraction(const WebCore::IntPoint& clientPosition, const WebCore::IntPoint& globalPosition);
     void requestAdditionalItemsForDragSession(const WebCore::IntPoint& clientPosition, const WebCore::IntPoint& globalPosition);
 #endif
-#endif
 
 #if !PLATFORM(COCOA) && !PLATFORM(WPE)
     static const char* interpretKeyEvent(const WebCore::KeyboardEvent*);
@@ -1087,11 +1099,13 @@ private:
 
     void mouseEvent(const WebMouseEvent&);
     void keyEvent(const WebKeyboardEvent&);
+
 #if ENABLE(IOS_TOUCH_EVENTS)
     void touchEventSync(const WebTouchEvent&, bool& handled);
 #elif ENABLE(TOUCH_EVENTS)
     void touchEvent(const WebTouchEvent&);
 #endif
+
 #if ENABLE(CONTEXT_MENUS)
     void contextMenuHidden() { m_isShowingContextMenu = false; }
     void contextMenuForKeyEvent();
@@ -1153,10 +1167,10 @@ private:
 #if PLATFORM(COCOA)
     void performDictionaryLookupAtLocation(const WebCore::FloatPoint&);
     void performDictionaryLookupOfCurrentSelection();
-    void performDictionaryLookupForRange(WebCore::Frame*, WebCore::Range&, NSDictionary *options, WebCore::TextIndicatorPresentationTransition);
-    WebCore::DictionaryPopupInfo dictionaryPopupInfoForRange(WebCore::Frame*, WebCore::Range&, NSDictionary **options, WebCore::TextIndicatorPresentationTransition);
+    void performDictionaryLookupForRange(WebCore::Frame&, WebCore::Range&, NSDictionary *options, WebCore::TextIndicatorPresentationTransition);
+    WebCore::DictionaryPopupInfo dictionaryPopupInfoForRange(WebCore::Frame&, WebCore::Range&, NSDictionary *options, WebCore::TextIndicatorPresentationTransition);
 #if ENABLE(PDFKIT_PLUGIN)
-    WebCore::DictionaryPopupInfo dictionaryPopupInfoForSelectionInPDFPlugin(PDFSelection *, PDFPlugin&, NSDictionary **options, WebCore::TextIndicatorPresentationTransition);
+    WebCore::DictionaryPopupInfo dictionaryPopupInfoForSelectionInPDFPlugin(PDFSelection *, PDFPlugin&, NSDictionary *options, WebCore::TextIndicatorPresentationTransition);
 #endif
 
     void windowAndViewFramesChanged(const WebCore::FloatRect& windowFrameInScreenCoordinates, const WebCore::FloatRect& windowFrameInUnflippedScreenCoordinates, const WebCore::FloatRect& viewFrameInWindowCoordinates, const WebCore::FloatPoint& accessibilityViewCoordinates);
@@ -1191,11 +1205,13 @@ private:
     void failedToShowPopupMenu();
 #endif
 
+    void didChooseFilesForOpenPanel(const Vector<String>&);
+    void didCancelForOpenPanel();
+
 #if PLATFORM(IOS)
     void didChooseFilesForOpenPanelWithDisplayStringAndIcon(const Vector<String>&, const String& displayString, const IPC::DataReference& iconData);
 #endif
-    void didChooseFilesForOpenPanel(const Vector<String>&);
-    void didCancelForOpenPanel();
+
 #if ENABLE(SANDBOX_EXTENSIONS)
     void extendSandboxForFilesFromOpenPanel(SandboxExtension::HandleArray&&);
 #endif
@@ -1209,23 +1225,26 @@ private:
     void userMediaAccessWasDenied(uint64_t userMediaID, uint64_t reason, String&& invalidConstraint);
 
     void didCompleteMediaDeviceEnumeration(uint64_t userMediaID, const Vector<WebCore::CaptureDevice>& devices, String&& deviceIdentifierHashSalt, bool originHasPersistentAccess);
-#if ENABLE(SANDBOX_EXTENSIONS)
+#endif
+
+#if ENABLE(MEDIA_STREAM) && ENABLE(SANDBOX_EXTENSIONS)
     void grantUserMediaDeviceSandboxExtensions(const MediaDeviceSandboxExtensions&);
     void revokeUserMediaDeviceSandboxExtensions(const Vector<String>&);
 #endif
-#endif
 
 #if ENABLE(WEB_RTC)
     void disableICECandidateFiltering();
     void enableICECandidateFiltering();
-#if USE(LIBWEBRTC)
+#endif
+
+#if ENABLE(WEB_RTC) && USE(LIBWEBRTC)
     void disableEnumeratingAllNetworkInterfaces();
     void enableEnumeratingAllNetworkInterfaces();
 #endif
-#endif
 
     void advanceToNextMisspelling(bool startBeforeSelection);
     void changeSpellingToWord(const String& word);
+
 #if USE(APPKIT)
     void uppercaseWord();
     void lowercaseWord();
@@ -1251,7 +1270,7 @@ private:
 
 #if PLATFORM(MAC)
     void performImmediateActionHitTestAtLocation(WebCore::FloatPoint);
-    RefPtr<WebCore::Range> lookupTextAtLocation(WebCore::FloatPoint, NSDictionary **options);
+    std::tuple<RefPtr<WebCore::Range>, RetainPtr<NSDictionary>> lookupTextAtLocation(WebCore::FloatPoint);
     void immediateActionDidUpdate();
     void immediateActionDidCancel();
     void immediateActionDidComplete();
@@ -1379,8 +1398,9 @@ private:
     UniqueRef<ViewGestureGeometryCollector> m_viewGestureGeometryCollector;
 
     RetainPtr<NSDictionary> m_dataDetectionContext;
+#endif
 
-#elif HAVE(ACCESSIBILITY) && PLATFORM(GTK)
+#if HAVE(ACCESSIBILITY) && PLATFORM(GTK)
     GRefPtr<WebPageAccessibilityObject> m_accessibilityObject;
 #endif
 
@@ -1392,7 +1412,7 @@ private:
 #if !PLATFORM(IOS)
     RefPtr<PageBanner> m_headerBanner;
     RefPtr<PageBanner> m_footerBanner;
-#endif // !PLATFORM(IOS)
+#endif
 
     RunLoop::Timer<WebPage> m_setCanStartMediaTimer;
     bool m_mayStartMediaWhenInWindow { false };
@@ -1422,19 +1442,25 @@ private:
     RefPtr<PlaybackSessionManager> m_playbackSessionManager;
     RefPtr<VideoFullscreenManager> m_videoFullscreenManager;
 #endif
+
 #if PLATFORM(IOS)
     bool m_allowsMediaDocumentInlinePlayback { false };
 #endif
+
 #if ENABLE(FULLSCREEN_API)
     RefPtr<WebFullScreenManager> m_fullScreenManager;
 #endif
+
     RefPtr<WebPopupMenu> m_activePopupMenu;
+
 #if ENABLE(CONTEXT_MENUS)
     RefPtr<WebContextMenu> m_contextMenu;
 #endif
+
 #if ENABLE(INPUT_TYPE_COLOR)
     WebColorChooser* m_activeColorChooser { nullptr };
 #endif
+
     RefPtr<WebOpenPanelResultListener> m_activeOpenPanelResultListener;
     RefPtr<NotificationPermissionRequestManager> m_notificationPermissionRequestManager;
 
@@ -1503,10 +1529,7 @@ private:
     RefPtr<WebCore::Node> m_interactionNode;
     WebCore::IntPoint m_lastInteractionLocation;
     
-    enum SelectionAnchor {
-        Start,
-        End
-    };
+    enum SelectionAnchor { Start, End };
     SelectionAnchor m_selectionAnchor { Start };
 
     RefPtr<WebCore::Node> m_potentialTapNode;
index 8bd97f6..d3eb4ae 100644 (file)
@@ -403,12 +403,12 @@ void WebPage::performDictionaryLookupAtLocation(const FloatPoint&)
     notImplemented();
 }
 
-void WebPage::performDictionaryLookupForSelection(Frame*, const VisibleSelection&, TextIndicatorPresentationTransition)
+void WebPage::performDictionaryLookupForSelection(Frame&, const VisibleSelection&, TextIndicatorPresentationTransition)
 {
     notImplemented();
 }
 
-void WebPage::performDictionaryLookupForRange(Frame*, Range&, NSDictionary *, TextIndicatorPresentationTransition)
+void WebPage::performDictionaryLookupForRange(Frame&, Range&, NSDictionary *, TextIndicatorPresentationTransition)
 {
     notImplemented();
 }
@@ -444,12 +444,12 @@ void WebPage::getSelectionContext(CallbackID callbackID)
 NSObject *WebPage::accessibilityObjectForMainFramePlugin()
 {
     if (!m_page)
-        return 0;
+        return nil;
     
-    if (PluginView* pluginView = pluginViewForFrame(&m_page->mainFrame()))
+    if (auto* pluginView = pluginViewForFrame(&m_page->mainFrame()))
         return pluginView->accessibilityObject();
     
-    return 0;
+    return nil;
 }
     
 void WebPage::registerUIProcessAccessibilityTokens(const IPC::DataReference& elementToken, const IPC::DataReference&)
@@ -1423,7 +1423,7 @@ static RefPtr<Range> rangeNearPositionMatchesText(const VisiblePosition& positio
 {
     auto range = Range::create(selectionRange->ownerDocument(), selectionRange->startPosition(), position.deepEquivalent().parentAnchoredEquivalent());
     unsigned targetOffset = TextIterator::rangeLength(range.ptr(), true);
-    return findClosestPlainText(*selectionRange.get(), matchText, 0, targetOffset);
+    return findClosestPlainText(*selectionRange.get(), matchText, { }, targetOffset);
 }
 
 void WebPage::getRectsAtSelectionOffsetWithText(int32_t offset, const String& text, CallbackID callbackID)
index 9b212dc..f39cae5 100644 (file)
@@ -199,7 +199,7 @@ using namespace WebKit;
     
     // Some plugins may be able to figure out the scroll position and inset on their own.
     bool applyContentOffset = true;
-    if (auto pluginView = m_page->pluginViewForFrame(m_page->mainFrame()))
+    if (auto pluginView = WebPage::pluginViewForFrame(m_page->mainFrame()))
         applyContentOffset = !pluginView->plugin()->pluginHandlesContentOffsetForAccessibilityHitTest();
 
     if (applyContentOffset) {
index 6c008f7..054c4fb 100644 (file)
@@ -174,12 +174,12 @@ void WebPage::handleAcceptedCandidate(WebCore::TextCheckingResult acceptedCandid
 NSObject *WebPage::accessibilityObjectForMainFramePlugin()
 {
     if (!m_page)
-        return 0;
+        return nil;
     
-    if (PluginView* pluginView = pluginViewForFrame(&m_page->mainFrame()))
+    if (auto* pluginView = pluginViewForFrame(&m_page->mainFrame()))
         return pluginView->accessibilityObject();
 
-    return 0;
+    return nil;
 }
 
 bool WebPage::shouldUsePDFPlugin() const
@@ -393,40 +393,41 @@ void WebPage::fontAtSelection(CallbackID callbackID)
     
 void WebPage::performDictionaryLookupAtLocation(const FloatPoint& floatPoint)
 {
-    if (PluginView* pluginView = pluginViewForFrame(&m_page->mainFrame())) {
+    if (auto* pluginView = pluginViewForFrame(&m_page->mainFrame())) {
         if (pluginView->performDictionaryLookupAtLocation(floatPoint))
             return;
     }
 
     // Find the frame the point is over.
-    IntPoint point = roundedIntPoint(floatPoint);
-    HitTestResult result = m_page->mainFrame().eventHandler().hitTestResultAtPoint(m_page->mainFrame().view()->windowToContents(point));
-    Frame* frame = result.innerNonSharedNode() ? result.innerNonSharedNode()->document().frame() : &m_page->focusController().focusedOrMainFrame();
-    NSDictionary *options = nil;
-    RefPtr<Range> range = DictionaryLookup::rangeAtHitTestResult(result, &options);
+    HitTestResult result = m_page->mainFrame().eventHandler().hitTestResultAtPoint(m_page->mainFrame().view()->windowToContents(roundedIntPoint(floatPoint)));
+    RetainPtr<NSDictionary> options;
+    auto range = DictionaryLookup::rangeAtHitTestResult(result, &options);
     if (!range)
         return;
 
-    performDictionaryLookupForRange(frame, *range, options, TextIndicatorPresentationTransition::Bounce);
+    auto* frame = result.innerNonSharedNode() ? result.innerNonSharedNode()->document().frame() : &m_page->focusController().focusedOrMainFrame();
+    if (!frame)
+        return;
+
+    performDictionaryLookupForRange(*frame, *range, options.get(), TextIndicatorPresentationTransition::Bounce);
 }
 
-void WebPage::performDictionaryLookupForSelection(Frame* frame, const VisibleSelection& selection, TextIndicatorPresentationTransition presentationTransition)
+void WebPage::performDictionaryLookupForSelection(Frame& frame, const VisibleSelection& selection, TextIndicatorPresentationTransition presentationTransition)
 {
-    NSDictionary *options = nil;
-    RefPtr<Range> selectedRange = DictionaryLookup::rangeForSelection(selection, &options);
-    if (selectedRange)
-        performDictionaryLookupForRange(frame, *selectedRange, options, presentationTransition);
+    RetainPtr<NSDictionary> options;
+    if (auto selectedRange = DictionaryLookup::rangeForSelection(selection, &options))
+        performDictionaryLookupForRange(frame, *selectedRange, options.get(), presentationTransition);
 }
 
 void WebPage::performDictionaryLookupOfCurrentSelection()
 {
-    Frame* frame = &m_page->focusController().focusedOrMainFrame();
-    performDictionaryLookupForSelection(frame, frame->selection().selection(), TextIndicatorPresentationTransition::BounceAndCrossfade);
+    auto& frame = m_page->focusController().focusedOrMainFrame();
+    performDictionaryLookupForSelection(frame, frame.selection().selection(), TextIndicatorPresentationTransition::BounceAndCrossfade);
 }
 
-DictionaryPopupInfo WebPage::dictionaryPopupInfoForRange(Frame* frame, Range& range, NSDictionary **options, TextIndicatorPresentationTransition presentationTransition)
+DictionaryPopupInfo WebPage::dictionaryPopupInfoForRange(Frame& frame, Range& range, NSDictionary *options, TextIndicatorPresentationTransition presentationTransition)
 {
-    Editor& editor = frame->editor();
+    Editor& editor = frame.editor();
     editor.setIsGettingDictionaryPopupInfo(true);
 
     DictionaryPopupInfo dictionaryPopupInfo;
@@ -442,12 +443,12 @@ DictionaryPopupInfo WebPage::dictionaryPopupInfoForRange(Frame* frame, Range& ra
         return dictionaryPopupInfo;
     }
 
-    IntRect rangeRect = frame->view()->contentsToWindow(quads[0].enclosingBoundingBox());
+    IntRect rangeRect = frame.view()->contentsToWindow(quads[0].enclosingBoundingBox());
 
     const RenderStyle* style = range.startContainer().renderStyle();
     float scaledAscent = style ? style->fontMetrics().ascent() * pageScaleFactor() : 0;
     dictionaryPopupInfo.origin = FloatPoint(rangeRect.x(), rangeRect.y() + scaledAscent);
-    dictionaryPopupInfo.options = *options;
+    dictionaryPopupInfo.options = options;
 
     NSAttributedString *nsAttributedString = editingAttributedStringFromRange(range, IncludeImagesInAttributedString::No);
 
@@ -485,7 +486,8 @@ DictionaryPopupInfo WebPage::dictionaryPopupInfoForRange(Frame* frame, Range& ra
 }
 
 #if ENABLE(PDFKIT_PLUGIN)
-DictionaryPopupInfo WebPage::dictionaryPopupInfoForSelectionInPDFPlugin(PDFSelection *selection, PDFPlugin& pdfPlugin, NSDictionary **options, WebCore::TextIndicatorPresentationTransition presentationTransition)
+
+DictionaryPopupInfo WebPage::dictionaryPopupInfoForSelectionInPDFPlugin(PDFSelection *selection, PDFPlugin& pdfPlugin, NSDictionary *options, WebCore::TextIndicatorPresentationTransition presentationTransition)
 {
     DictionaryPopupInfo dictionaryPopupInfo;
     if (!selection.string.length)
@@ -532,18 +534,18 @@ DictionaryPopupInfo WebPage::dictionaryPopupInfoForSelectionInPDFPlugin(PDFSelec
     dataForSelection.presentationTransition = presentationTransition;
     
     dictionaryPopupInfo.origin = rangeRect.origin;
-    dictionaryPopupInfo.options = *options;
+    dictionaryPopupInfo.options = options;
     dictionaryPopupInfo.textIndicator = dataForSelection;
     dictionaryPopupInfo.attributedString = scaledNSAttributedString;
     
     return dictionaryPopupInfo;
 }
+
 #endif
 
-void WebPage::performDictionaryLookupForRange(Frame* frame, Range& range, NSDictionary *options, TextIndicatorPresentationTransition presentationTransition)
+void WebPage::performDictionaryLookupForRange(Frame& frame, Range& range, NSDictionary *options, TextIndicatorPresentationTransition presentationTransition)
 {
-    DictionaryPopupInfo dictionaryPopupInfo = dictionaryPopupInfoForRange(frame, range, &options, presentationTransition);
-    send(Messages::WebPageProxy::DidPerformDictionaryLookup(dictionaryPopupInfo));
+    send(Messages::WebPageProxy::DidPerformDictionaryLookup(dictionaryPopupInfoForRange(frame, range, options, presentationTransition)));
 }
 
 bool WebPage::performNonEditingBehaviorForSelector(const String& selector, KeyboardEvent* event)
@@ -996,20 +998,18 @@ void WebPage::performImmediateActionHitTestAtLocation(WebCore::FloatPoint locati
     RefPtr<Range> selectionRange = corePage()->focusController().focusedOrMainFrame().selection().selection().firstRange();
 
     URL absoluteLinkURL = hitTestResult.absoluteLinkURL();
-    Element *URLElement = hitTestResult.URLElement();
-    if (!absoluteLinkURL.isEmpty() && URLElement) {
-        RefPtr<Range> linkRange = rangeOfContents(*URLElement);
-        immediateActionResult.linkTextIndicator = TextIndicator::createWithRange(*linkRange, TextIndicatorOptionUseBoundingRectAndPaintAllContentForComplexRanges, TextIndicatorPresentationTransition::FadeIn);
-    }
-
-    NSDictionary *options = nil;
-    RefPtr<Range> lookupRange = lookupTextAtLocation(locationInViewCoordinates, &options);
-    immediateActionResult.lookupText = lookupRange ? lookupRange->text() : String();
-
-    if (lookupRange) {
-        if (Node* node = hitTestResult.innerNode()) {
-            if (Frame* hitTestResultFrame = node->document().frame())
-                immediateActionResult.dictionaryPopupInfo = dictionaryPopupInfoForRange(hitTestResultFrame, *lookupRange.get(), &options, TextIndicatorPresentationTransition::FadeIn);
+    Element* URLElement = hitTestResult.URLElement();
+    if (!absoluteLinkURL.isEmpty() && URLElement)
+        immediateActionResult.linkTextIndicator = TextIndicator::createWithRange(rangeOfContents(*URLElement), TextIndicatorOptionUseBoundingRectAndPaintAllContentForComplexRanges, TextIndicatorPresentationTransition::FadeIn);
+
+    auto lookupResult = lookupTextAtLocation(locationInViewCoordinates);
+    if (auto* lookupRange = std::get<RefPtr<Range>>(lookupResult).get()) {
+        immediateActionResult.lookupText = lookupRange->text();
+        if (auto* node = hitTestResult.innerNode()) {
+            if (auto* frame = node->document().frame()) {
+                auto options = std::get<RetainPtr<NSDictionary>>(lookupResult).get();
+                immediateActionResult.dictionaryPopupInfo = dictionaryPopupInfoForRange(*frame, *lookupRange, options, TextIndicatorPresentationTransition::FadeIn);
+            }
         }
     }
 
@@ -1053,29 +1053,27 @@ void WebPage::performImmediateActionHitTestAtLocation(WebCore::FloatPoint locati
     }
 
 #if ENABLE(PDFKIT_PLUGIN)
-    // See if we have a PDF
-    if (element && is<HTMLPlugInImageElement>(*element)) {
-        HTMLPlugInImageElement& pluginImageElement = downcast<HTMLPlugInImageElement>(*element);
-        PluginView* pluginView = reinterpret_cast<PluginView*>(pluginImageElement.pluginWidget());
-        Plugin* plugin = pluginView ? pluginView->plugin() : nullptr;
-        if (is<PDFPlugin>(plugin)) {
-            PDFPlugin* pdfPugin = downcast<PDFPlugin>(plugin);
-            // FIXME: We don't have API to identify images inside PDFs based on position.
-            NSDictionary *options = nil;
-            PDFSelection *selection = nil;
-            String selectedText = pdfPugin->lookupTextAtLocation(locationInViewCoordinates, immediateActionResult, &selection, &options);
-            if (!selectedText.isEmpty()) {
-                if (element->document().isPluginDocument()) {
-                    // FIXME(144030): Focus does not seem to get set to the PDF when invoking the menu.
-                    PluginDocument& pluginDocument = static_cast<PluginDocument&>(element->document());
-                    pluginDocument.setFocusedElement(element);
+    if (is<HTMLPlugInImageElement>(element)) {
+        if (auto* pluginView = static_cast<PluginView*>(downcast<HTMLPlugInImageElement>(*element).pluginWidget())) {
+            if (is<PDFPlugin>(pluginView->plugin())) {
+                // FIXME: We don't have API to identify images inside PDFs based on position.
+                auto& plugIn = downcast<PDFPlugin>(*pluginView->plugin());
+                auto lookupResult = plugIn.lookupTextAtLocation(locationInViewCoordinates, immediateActionResult);
+                auto lookupText = std::get<String>(lookupResult);
+                if (!lookupText.isEmpty()) {
+                    // FIXME (144030): Focus does not seem to get set to the PDF when invoking the menu.
+                    auto& document = element->document();
+                    if (is<PluginDocument>(document))
+                        downcast<PluginDocument>(document).setFocusedElement(element);
+
+                    auto selection = std::get<RetainPtr<PDFSelection>>(lookupResult).get();
+                    auto options = std::get<RetainPtr<NSDictionary>>(lookupResult).get();
+
+                    immediateActionResult.lookupText = lookupText;
+                    immediateActionResult.isTextNode = true;
+                    immediateActionResult.isSelected = true;
+                    immediateActionResult.dictionaryPopupInfo = dictionaryPopupInfoForSelectionInPDFPlugin(selection, plugIn, options, TextIndicatorPresentationTransition::FadeIn);
                 }
-
-                immediateActionResult.lookupText = selectedText;
-                immediateActionResult.isTextNode = true;
-                immediateActionResult.isSelected = true;
-
-                immediateActionResult.dictionaryPopupInfo = dictionaryPopupInfoForSelectionInPDFPlugin(selection, *pdfPugin, &options, TextIndicatorPresentationTransition::FadeIn);
             }
         }
     }
@@ -1087,15 +1085,17 @@ void WebPage::performImmediateActionHitTestAtLocation(WebCore::FloatPoint locati
     send(Messages::WebPageProxy::DidPerformImmediateActionHitTest(immediateActionResult, immediateActionHitTestPreventsDefault, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())));
 }
 
-RefPtr<WebCore::Range> WebPage::lookupTextAtLocation(FloatPoint locationInViewCoordinates, NSDictionary **options)
+std::tuple<RefPtr<WebCore::Range>, RetainPtr<NSDictionary>> WebPage::lookupTextAtLocation(FloatPoint locationInViewCoordinates)
 {
-    MainFrame& mainFrame = corePage()->mainFrame();
+    auto& mainFrame = corePage()->mainFrame();
     if (!mainFrame.view() || !mainFrame.view()->renderView())
-        return nullptr;
+        return { nullptr, nil };
 
-    IntPoint point = roundedIntPoint(locationInViewCoordinates);
-    HitTestResult result = mainFrame.eventHandler().hitTestResultAtPoint(m_page->mainFrame().view()->windowToContents(point));
-    return DictionaryLookup::rangeAtHitTestResult(result, options);
+    auto point = roundedIntPoint(locationInViewCoordinates);
+    auto result = mainFrame.eventHandler().hitTestResultAtPoint(m_page->mainFrame().view()->windowToContents(point));
+    RetainPtr<NSDictionary> options;
+    auto range = DictionaryLookup::rangeAtHitTestResult(result, &options);
+    return { WTFMove(range), WTFMove(options) };
 }
 
 void WebPage::immediateActionDidUpdate()
index ffc29f2..04748de 100644 (file)
@@ -1,3 +1,15 @@
+2017-11-23  Darin Adler  <darin@apple.com>
+
+        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.
+
+        * WebView/WebImmediateActionController.mm:
+        (-[WebImmediateActionController _animationControllerForText]): Use RetainPtr so we don't leak.
+        * WebView/WebView.mm:
+        (coreOptions): Use |= instead of | to build up FindOptions.
+
 2017-11-23  Sam Weinig  <sam@webkit.org>
 
         Remove unneeded ScriptController::processingUserGesture() forwarding functions
index ef4bf8b..ddf3f2d 100644 (file)
@@ -564,12 +564,12 @@ static IntRect elementBoundingBoxInWindowCoordinatesFromNode(Node* node)
     if (!frame)
         return nil;
 
-    NSDictionary *options = nil;
-    RefPtr<Range> dictionaryRange = DictionaryLookup::rangeAtHitTestResult(_hitTestResult, &options);
+    RetainPtr<NSDictionary> options;
+    auto dictionaryRange = DictionaryLookup::rangeAtHitTestResult(_hitTestResult, &options);
     if (!dictionaryRange)
         return nil;
 
-    DictionaryPopupInfo dictionaryPopupInfo = [WebImmediateActionController _dictionaryPopupInfoForRange:*dictionaryRange inFrame:frame withLookupOptions:options indicatorOptions:TextIndicatorOptionDefault transition: TextIndicatorPresentationTransition::FadeIn];
+    auto dictionaryPopupInfo = [WebImmediateActionController _dictionaryPopupInfoForRange:*dictionaryRange inFrame:frame withLookupOptions:options.get() indicatorOptions:TextIndicatorOptionDefault transition: TextIndicatorPresentationTransition::FadeIn];
     if (!dictionaryPopupInfo.attributedString)
         return nil;
 
index 445778d..0653ca8 100644 (file)
@@ -566,12 +566,20 @@ static const char webViewIsOpen[] = "At least one WebView is still open.";
 
 FindOptions coreOptions(WebFindOptions options)
 {
-    return (options & WebFindOptionsCaseInsensitive ? CaseInsensitive : 0)
-        | (options & WebFindOptionsAtWordStarts ? AtWordStarts : 0)
-        | (options & WebFindOptionsTreatMedialCapitalAsWordStart ? TreatMedialCapitalAsWordStart : 0)
-        | (options & WebFindOptionsBackwards ? Backwards : 0)
-        | (options & WebFindOptionsWrapAround ? WrapAround : 0)
-        | (options & WebFindOptionsStartInSelection ? StartInSelection : 0);
+    FindOptions findOptions;
+    if (options & WebFindOptionsCaseInsensitive)
+        findOptions |= CaseInsensitive;
+    if (options & WebFindOptionsAtWordStarts)
+        findOptions |= AtWordStarts;
+    if (options & WebFindOptionsTreatMedialCapitalAsWordStart)
+        findOptions |= TreatMedialCapitalAsWordStart;
+    if (options & WebFindOptionsBackwards)
+        findOptions |= Backwards;
+    if (options & WebFindOptionsWrapAround)
+        findOptions |= WrapAround;
+    if (options & WebFindOptionsStartInSelection)
+        findOptions |= StartInSelection;
+    return findOptions;
 }
 
 LayoutMilestones coreLayoutMilestones(WebLayoutMilestones milestones)
index f508356..ff39e58 100644 (file)
@@ -1,5 +1,18 @@
 2017-11-23  Darin Adler  <darin@apple.com>
 
+        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.
+
+        * 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.
+
+2017-11-23  Darin Adler  <darin@apple.com>
+
         Reduce WTF::String operations that do unnecessary Unicode operations instead of ASCII
         https://bugs.webkit.org/show_bug.cgi?id=179907
 
index ef87123..4ec3a72 100644 (file)
@@ -3824,7 +3824,13 @@ HRESULT WebView::searchFor(_In_ BSTR str, BOOL forward, BOOL caseFlag, BOOL wrap
     if (!str || !SysStringLen(str))
         return E_INVALIDARG;
 
-    FindOptions options = (caseFlag ? 0 : CaseInsensitive) | (forward ? 0 : Backwards) | (wrapFlag ? WrapAround : 0);
+    FindOptions options;
+    if (!caseFlag)
+        options |= CaseInsensitive;
+    if (!forward)
+        options |= Backwards;
+    if (wrapFlag)
+        options |= WrapAround;
     *found = m_page->findString(toString(str), options);
     return S_OK;
 }
@@ -3887,7 +3893,11 @@ HRESULT WebView::markAllMatchesForText(_In_ BSTR str, BOOL caseSensitive, BOOL h
     if (!str || !SysStringLen(str))
         return E_INVALIDARG;
 
-    *matches = m_page->markAllMatchesForText(toString(str), caseSensitive ? 0 : WebCore::CaseInsensitive, highlight, limit);
+    WebCore::FindOptions options;
+    if (!caseSensitive)
+        options |= WebCore::CaseInsensitive;
+
+    *matches = m_page->markAllMatchesForText(toString(str), options, highlight, limit);
     return S_OK;
 }
 
@@ -7786,7 +7796,22 @@ HRESULT WebView::findString(_In_ BSTR string, WebFindOptions options, _Deref_opt
     if (!found)
         return E_POINTER;
 
-    *found = m_page->findString(toString(string), options);
+    WebCore::FindOptions coreOptions;
+
+    if (options & WebFindOptionsCaseInsensitive)
+        coreOptions |= WebCore::CaseInsensitive;
+    if (options & WebFindOptionsAtWordStarts)
+        coreOptions |= WebCore::AtWordStarts;
+    if (options & WebFindOptionsTreatMedialCapitalAsWordStart)
+        coreOptions |= WebCore::TreatMedialCapitalAsWordStart;
+    if (options & WebFindOptionsBackwards)
+        coreOptions |= WebCore::Backwards;
+    if (options & WebFindOptionsWrapAround)
+        coreOptions |= WebCore::WrapAround;
+    if (options & WebFindOptionsStartInSelection)
+        coreOptions |= WebCore::StartInSelection;
+
+    *found = m_page->findString(toString(string), coreOptions);
     return S_OK;
 }