Make explicit which TextIndicator animations are driven manually, and which run autom...
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Jan 2015 19:09:31 +0000 (19:09 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Jan 2015 19:09:31 +0000 (19:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=140113
<rdar://problem/19383425>

Reviewed by Darin Adler.

* UIProcess/mac/PageClientImpl.mm:
(WebKit::PageClientImpl::didPerformDictionaryLookup):
Delete an inaccurate comment.

* page/TextIndicator.cpp:
(WebCore::TextIndicator::wantsManualAnimation):
Add wantsManualAnimation(). The old transitions (Bounce and BounceAndCrossfade)
run automatically, and the new ones (FadeIn and Crossfade) are driven manually.

* page/TextIndicator.h:
Add a comment explaining which animations are manual and which are not.

* page/mac/TextIndicatorWindow.mm:
(-[WebTextIndicatorView present]):
Make use of wantsManualAnimation(). The previous (wantsFadeIn || wantsCrossfade)
was wrong, because it was true for BounceAndCrossfade, and would cause
BounceAndCrossfade animations (which aren't driven manually) to stall at progress=0.

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

Source/WebCore/ChangeLog
Source/WebCore/page/TextIndicator.cpp
Source/WebCore/page/TextIndicator.h
Source/WebCore/page/mac/TextIndicatorWindow.mm
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/mac/PageClientImpl.mm

index 89f9aa2..27c23b2 100644 (file)
@@ -1,3 +1,25 @@
+2015-01-06  Timothy Horton  <timothy_horton@apple.com>
+
+        Make explicit which TextIndicator animations are driven manually, and which run automatically
+        https://bugs.webkit.org/show_bug.cgi?id=140113
+        <rdar://problem/19383425>
+
+        Reviewed by Darin Adler.
+
+        * page/TextIndicator.cpp:
+        (WebCore::TextIndicator::wantsManualAnimation):
+        Add wantsManualAnimation(). The old transitions (Bounce and BounceAndCrossfade)
+        run automatically, and the new ones (FadeIn and Crossfade) are driven manually.
+
+        * page/TextIndicator.h:
+        Add a comment explaining which animations are manual and which are not.
+
+        * page/mac/TextIndicatorWindow.mm:
+        (-[WebTextIndicatorView present]):
+        Make use of wantsManualAnimation(). The previous (wantsFadeIn || wantsCrossfade)
+        was wrong, because it was true for BounceAndCrossfade, and would cause
+        BounceAndCrossfade animations (which aren't driven manually) to stall at progress=0.
+
 2015-01-06  Anders Carlsson  <andersca@apple.com>
 
         Give empty pages a valid database provider.
index 362b9c4..b1d131b 100644 (file)
@@ -207,7 +207,8 @@ bool TextIndicator::wantsBounce() const
     case TextIndicatorPresentationTransition::None:
         return false;
     }
-    
+
+    ASSERT_NOT_REACHED();
     return false;
 }
 
@@ -226,7 +227,8 @@ bool TextIndicator::wantsContentCrossfade() const
     case TextIndicatorPresentationTransition::None:
         return false;
     }
-    
+
+    ASSERT_NOT_REACHED();
     return false;
 }
 
@@ -242,7 +244,25 @@ bool TextIndicator::wantsFadeIn() const
     case TextIndicatorPresentationTransition::None:
         return false;
     }
-    
+
+    ASSERT_NOT_REACHED();
+    return false;
+}
+
+bool TextIndicator::wantsManualAnimation() const
+{
+    switch (m_data.presentationTransition) {
+    case TextIndicatorPresentationTransition::FadeIn:
+    case TextIndicatorPresentationTransition::Crossfade:
+        return true;
+
+    case TextIndicatorPresentationTransition::Bounce:
+    case TextIndicatorPresentationTransition::BounceAndCrossfade:
+    case TextIndicatorPresentationTransition::None:
+        return false;
+    }
+
+    ASSERT_NOT_REACHED();
     return false;
 }
 
index 0844bd5..4ea197d 100644 (file)
@@ -46,8 +46,12 @@ class Range;
 
 enum class TextIndicatorPresentationTransition {
     None,
+
+    // These animations drive themselves.
     Bounce,
     BounceAndCrossfade,
+
+    // These animations need to be driven manually via TextIndicatorWindow::setAnimationProgress.
     FadeIn,
     Crossfade
 };
@@ -85,6 +89,7 @@ public:
     bool wantsBounce() const;
     bool wantsContentCrossfade() const;
     bool wantsFadeIn() const;
+    bool wantsManualAnimation() const;
 
 private:
     TextIndicator(const TextIndicatorData&);
index 478ab4d..b4a7784 100644 (file)
@@ -275,7 +275,7 @@ static RetainPtr<CABasicAnimation> createFadeInAnimation(CFTimeInterval duration
 
     [CATransaction begin];
     for (CALayer *bounceLayer in _bounceLayers.get()) {
-        if (wantsFadeIn || wantsCrossfade)
+        if (_textIndicator->wantsManualAnimation())
             bounceLayer.speed = 0;
 
         if (!wantsFadeIn)
index 7220608..869959d 100644 (file)
@@ -1,3 +1,15 @@
+2015-01-06  Timothy Horton  <timothy_horton@apple.com>
+
+        Make explicit which TextIndicator animations are driven manually, and which run automatically
+        https://bugs.webkit.org/show_bug.cgi?id=140113
+        <rdar://problem/19383425>
+
+        Reviewed by Darin Adler.
+
+        * UIProcess/mac/PageClientImpl.mm:
+        (WebKit::PageClientImpl::didPerformDictionaryLookup):
+        Delete an inaccurate comment.
+
 2015-01-06  Anders Carlsson  <andersca@apple.com>
 
         Remove now unused IndexedDB code
index ef9e8b3..88abd4f 100644 (file)
@@ -569,9 +569,6 @@ void PageClientImpl::didPerformDictionaryLookup(const DictionaryPopupInfo& dicti
     RetainPtr<NSMutableDictionary> mutableOptions = adoptNS([(NSDictionary *)dictionaryPopupInfo.options.get() mutableCopy]);
 
     if (canLoadLUTermOptionDisableSearchTermIndicator() && dictionaryPopupInfo.textIndicator.contentImage) {
-        // Run the animations serially because attaching another subwindow breaks the bounce animation.
-        // We could consider making the bounce NSAnimationNonblockingThreaded instead, which seems
-        // to work, but need to consider all of the implications.
         [m_wkView _setTextIndicator:TextIndicator::create(dictionaryPopupInfo.textIndicator) fadeOut:NO];
         [mutableOptions setObject:@YES forKey:getLUTermOptionDisableSearchTermIndicator()];
         [getLULookupDefinitionModuleClass() showDefinitionForTerm:dictionaryPopupInfo.attributedString.string.get() atLocation:textBaselineOrigin options:mutableOptions.get()];