Title: [182088] trunk/Source/WebCore
Revision
182088
Author
timothy_hor...@apple.com
Date
2015-03-27 16:01:37 -0700 (Fri, 27 Mar 2015)

Log Message

WebProcess started by editable WKWebView spends 15% of its initialization time loading DataDetectors
https://bugs.webkit.org/show_bug.cgi?id=143142
<rdar://problem/20324495>

Reviewed by Anders Carlsson.

Calling DataDetectorsLibrary() is expensive; we should avoid doing it
until actually necessary. When loading a page that makes a caret selection,
ServicesOverlayController was calling DataDetectorsLibrary() (ignoring the fact
that a caret selection can't have any services associated with it) to avoid
crashing on systems where DataDetectors is not available. Instead, we should
first check if there's anything to do, and then check for the existence
of DataDetectors.

* page/mac/ServicesOverlayController.mm:
(WebCore::ServicesOverlayController::buildPhoneNumberHighlights):
Build the list of phone number ranges, and bail (clearing the potential highlights)
if it is empty, before calling DataDetectorsLibrary().

(WebCore::ServicesOverlayController::buildSelectionHighlight):
Check the list of selection rects, and bail (clearing the potential highlights)
if it is empty, before calling DataDetectorsLibrary().

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (182087 => 182088)


--- trunk/Source/WebCore/ChangeLog	2015-03-27 22:56:15 UTC (rev 182087)
+++ trunk/Source/WebCore/ChangeLog	2015-03-27 23:01:37 UTC (rev 182088)
@@ -1,3 +1,28 @@
+2015-03-27  Tim Horton  <timothy_hor...@apple.com>
+
+        WebProcess started by editable WKWebView spends 15% of its initialization time loading DataDetectors
+        https://bugs.webkit.org/show_bug.cgi?id=143142
+        <rdar://problem/20324495>
+
+        Reviewed by Anders Carlsson.
+
+        Calling DataDetectorsLibrary() is expensive; we should avoid doing it
+        until actually necessary. When loading a page that makes a caret selection,
+        ServicesOverlayController was calling DataDetectorsLibrary() (ignoring the fact
+        that a caret selection can't have any services associated with it) to avoid
+        crashing on systems where DataDetectors is not available. Instead, we should
+        first check if there's anything to do, and then check for the existence
+        of DataDetectors.
+
+        * page/mac/ServicesOverlayController.mm:
+        (WebCore::ServicesOverlayController::buildPhoneNumberHighlights):
+        Build the list of phone number ranges, and bail (clearing the potential highlights)
+        if it is empty, before calling DataDetectorsLibrary().
+
+        (WebCore::ServicesOverlayController::buildSelectionHighlight):
+        Check the list of selection rects, and bail (clearing the potential highlights)
+        if it is empty, before calling DataDetectorsLibrary().
+
 2015-03-27  Jer Noble  <jer.no...@apple.com>
 
         [Mac] Safari fails to fire page "load" event with video[preload=none]

Modified: trunk/Source/WebCore/page/mac/ServicesOverlayController.mm (182087 => 182088)


--- trunk/Source/WebCore/page/mac/ServicesOverlayController.mm	2015-03-27 22:56:15 UTC (rev 182087)
+++ trunk/Source/WebCore/page/mac/ServicesOverlayController.mm	2015-03-27 23:01:37 UTC (rev 182088)
@@ -471,37 +471,44 @@
 
 void ServicesOverlayController::buildPhoneNumberHighlights()
 {
-    if (!DataDetectorsLibrary())
+    if (!m_mainFrame.settings().serviceControlsEnabled())
         return;
 
-    if (!m_mainFrame.settings().serviceControlsEnabled())
+    Vector<RefPtr<Range>> phoneNumberRanges;
+    for (Frame* frame = &m_mainFrame; frame; frame = frame->tree().traverseNext())
+        phoneNumberRanges.appendVector(frame->editor().detectedTelephoneNumberRanges());
+
+    if (phoneNumberRanges.isEmpty()) {
+        removeAllPotentialHighlightsOfType(Highlight::Type::TelephoneNumber);
+        didRebuildPotentialHighlights();
         return;
+    }
 
+    if (!DataDetectorsLibrary())
+        return;
+
     HashSet<RefPtr<Highlight>> newPotentialHighlights;
 
     FrameView& mainFrameView = *m_mainFrame.view();
 
-    for (Frame* frame = &m_mainFrame; frame; frame = frame->tree().traverseNext()) {
-        auto& ranges = frame->editor().detectedTelephoneNumberRanges();
-        for (auto& range : ranges) {
-            // FIXME: This will choke if the range wraps around the edge of the view.
-            // What should we do in that case?
-            IntRect rect = textQuadsToBoundingRectForRange(*range);
+    for (auto& range : phoneNumberRanges) {
+        // FIXME: This will choke if the range wraps around the edge of the view.
+        // What should we do in that case?
+        IntRect rect = textQuadsToBoundingRectForRange(*range);
 
-            // Convert to the main document's coordinate space.
-            // FIXME: It's a little crazy to call contentsToWindow and then windowToContents in order to get the right coordinate space.
-            // We should consider adding conversion functions to ScrollView for contentsToDocument(). Right now, contentsToRootView() is
-            // not equivalent to what we need when you have a topContentInset or a header banner.
-            FrameView* viewForRange = range->ownerDocument().view();
-            if (!viewForRange)
-                continue;
-            rect.setLocation(mainFrameView.windowToContents(viewForRange->contentsToWindow(rect.location())));
+        // Convert to the main document's coordinate space.
+        // FIXME: It's a little crazy to call contentsToWindow and then windowToContents in order to get the right coordinate space.
+        // We should consider adding conversion functions to ScrollView for contentsToDocument(). Right now, contentsToRootView() is
+        // not equivalent to what we need when you have a topContentInset or a header banner.
+        FrameView* viewForRange = range->ownerDocument().view();
+        if (!viewForRange)
+            continue;
+        rect.setLocation(mainFrameView.windowToContents(viewForRange->contentsToWindow(rect.location())));
 
-            CGRect cgRect = rect;
-            RetainPtr<DDHighlightRef> ddHighlight = adoptCF(DDHighlightCreateWithRectsInVisibleRectWithStyleAndDirection(nullptr, &cgRect, 1, mainFrameView.visibleContentRect(), DDHighlightStyleBubbleStandard | DDHighlightStyleStandardIconArrow, YES, NSWritingDirectionNatural, NO, YES));
+        CGRect cgRect = rect;
+        RetainPtr<DDHighlightRef> ddHighlight = adoptCF(DDHighlightCreateWithRectsInVisibleRectWithStyleAndDirection(nullptr, &cgRect, 1, mainFrameView.visibleContentRect(), DDHighlightStyleBubbleStandard | DDHighlightStyleStandardIconArrow, YES, NSWritingDirectionNatural, NO, YES));
 
-            newPotentialHighlights.add(Highlight::createForTelephoneNumber(*this, ddHighlight, range));
-        }
+        newPotentialHighlights.add(Highlight::createForTelephoneNumber(*this, ddHighlight, range));
     }
 
     replaceHighlightsOfTypePreservingEquivalentHighlights(newPotentialHighlights, Highlight::Type::TelephoneNumber);
@@ -511,16 +518,22 @@
 
 void ServicesOverlayController::buildSelectionHighlight()
 {
-    if (!DataDetectorsLibrary())
+    if (!m_mainFrame.settings().serviceControlsEnabled())
         return;
 
-    if (!m_mainFrame.settings().serviceControlsEnabled())
+    if (m_currentSelectionRects.isEmpty()) {
+        removeAllPotentialHighlightsOfType(Highlight::Type::Selection);
+        didRebuildPotentialHighlights();
         return;
+    }
 
     Page* page = m_mainFrame.page();
     if (!page)
         return;
 
+    if (!DataDetectorsLibrary())
+        return;
+
     HashSet<RefPtr<Highlight>> newPotentialHighlights;
 
     Vector<CGRect> cgRects;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to