WebProcess started by editable WKWebView spends 15% of its initialization time loadin...

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

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@182088 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index d428012..cc22990 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,28 @@
+2015-03-27  Tim Horton  <timothy_horton@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.noble@apple.com>
 
         [Mac] Safari fails to fire page "load" event with video[preload=none]

diff --git a/Source/WebCore/page/mac/ServicesOverlayController.mm b/Source/WebCore/page/mac/ServicesOverlayController.mm
index d51ca59..ebb633d 100644 (file)
--- a/Source/WebCore/page/mac/ServicesOverlayController.mm
+++ b/Source/WebCore/page/mac/ServicesOverlayController.mm
@@ -471,37 +471,44 @@ void ServicesOverlayController::removeAllPotentialHighlightsOfType(Highlight::Ty
 
 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);
-
-            // 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())));
+    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())));
 
-            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::buildPhoneNumberHighlights()
 
 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;