Situation: we need to animate the scroll position adjustment for (say) virtual keyboard showing/hiding Solution: Right now in the blackberry port we use ScrollableArea::ScrollAnimator::scroll() method. Basically we: 1) calculate the scroll adjustment needed 2) set ScrollableArea::m_constrainsScrollingToContentEdge to 'false', so we can "overscroll" whenever needed in order to properly show the focused element 3) call main frame's ScrollableArea::scrollAnimator::scroll() 4) set ScrollableArea::m_constrainsScrollingToContentEdge back to 'true' Problem: when we call (3), all the scroll animation is timer-based, so it happens asynchronously, whereas (4) happens right after (3) synchronously. This way when (3) is executing, we have already reset m_constrainsScrollingToContentEdge. We need a way to tell ScrollableArea::ScrollAnimator::scroll() to execute something when it finishes animating, or to tell the caller when it finished.
Created attachment 138847 [details] patch v1
Comment on attachment 138847 [details] patch v1 will do it differently after talking to Andersca
Created attachment 138929 [details] patch v2 Adds ScrollAnimBB class, and add the customization to allow overscrolling while the animation runs in there (as per andersca suggestion)
Attachment 138929 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/W..." exit_code: 1 Source/WebCore/platform/blackberry/ScrollAnimatorBlackBerry.cpp:6: You should add a blank line after implementation file's own header. [build/include_order] [4] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 138999 [details] (r115347, r=andersca) patch v3 Fixed style and build system issues
some context: 1 <andersca> tonikitoo: hmm, why do you need to overscroll when showing the keyboard? is it because the keyboard is on top of the web view somehow? 2 <tonikitoo> andersca, exactly. when we show a Virtual Keyboard, we do not set the viewport size (trying to avoid layouts, and stuff) 3 <andersca> tonikitoo: do you disable scripts as well? 4 <tonikitoo> we have a different way of controlling it 5 <tonikitoo> andersca, not sure I followed the scripts part 6 <andersca> tonikitoo: does showing the keyboard prevent script execution 7 <tonikitoo> andersca, no 8 <andersca> tonikitoo: so you could have a script that scrolls and wreaks havoc 9 <tonikitoo> andersca, that script could exist, yes. And no, so far we are using the ScrollAnimNone, but in the future we will need a custom one 10 <andersca> tonikitoo: do you have a custom blackberry scroll animator? 11 <andersca> tonikitoo: so if you need a custom one then maybe you should just put the logic there 12 <andersca> tonikitoo: since it seems specialized 13 <tonikitoo> andersca, I can add ScrollAnimBlackberry and inherit from ScrollAnimNone. but the base classes would still need a stub of the method right? 14 <tonikitoo> agree that is seems like too specialized 15 <andersca> tonikitoo: then in your scroll function you could allow over scrolling and when you know the animation is finished you can disable it again 16 <tonikitoo> andersca, but for now I would rather still use scrollanimatornone::scroll 17 <tonikitoo> andersca, maybe I should add animationFinished method instead 18 <tonikitoo> and do the logic from there 19 <andersca> tonikitoo: just make your scroll animator inherit from scrollanimatornone :) 20 <andersca> tonikitoo: then you can add animationFinished to scrollanimatornone 21 <tonikitoo> andersca, exactly, that is what I thought too 22 <tonikitoo> andersca, do you think we would not need the setDisableConstrainsScrollingToContentEdgeWhileAnimating method then? 23 <andersca> tonikitoo: i don't think that would be needed, as long as you'd inherit from ScrollAnimatorNone 24 <tonikitoo> andersca, but I want specific scroll animation runs to allow overscrolling 25 <andersca> tonikitoo: oh, i see 26 <andersca> hmm 27 <tonikitoo> so if I do ScrollView::setContrainsScrollingToContentEdge(false); ScrollView::scrollAnimator()->scroll() ... I want to be able to toggle the overscroll flag OFF , or leave it as ON, depending on the case. 28 <andersca> tonikitoo: yeah i suppose 29 <andersca> it's still pretty specialized :/ 30 <andersca> tonikitoo: where do you call scroll from 31 <tonikitoo> andersca, so far it will be used for animating the scroll position adjustment as we show the virtual keyboard only. 32 <tonikitoo> so it does not have other use cases 33 <andersca> tonikitoo: and where's that called from 34 <tonikitoo> let me paste the code to you 35 <andersca> tonikitoo: couldn't you cast the scroll animator to your specific subclass and call your setter from there? 36 <tonikitoo> andersca, it looks like this: 37 <tonikitoo> 640 // In order to adjust the scroll position to ensure the focused input field is visible, 38 <tonikitoo> 641 // we allow overscrolling. However this overscroll has to be strictly allowed towards the 39 <tonikitoo> 642 // bottom of the page on the y axis only, where the virtual keyboard pops up from. 40 <tonikitoo> 643 WebCore::IntPoint destinationScrollOffset = revealRect.location(); 41 <tonikitoo> 644 destinationScrollOffset.clampNegativeToZero(); 42 <tonikitoo> 645 WebCore::IntPoint maximumScrollPosition = WebCore::IntPoint(mainFrameView->contentsWidth() - actualScreenRect.width(), mainFrameView->contentsHeight() - actualScreenRect.heig ht()); 43 <tonikitoo> 646 destinationScrollOffset = destinationScrollOffset.shrunkTo(maximumScrollPosition); 44 <tonikitoo> 647 45 <tonikitoo> 648 // Animate the scroll position adjustment. 46 <tonikitoo> 649 WebCore::IntPoint initialScrollOffset = mainFrameView->scrollPosition(); 47 <tonikitoo> 650 mainFrameView->scrollAnimator()->setDisableConstrainsScrollingToContentEdgeWhileAnimating(true); 48 <tonikitoo> 651 mainFrameView->scrollAnimator()->scroll(VerticalScrollbar, ScrollByPixel, 1, (destinationScrollOffset - initialScrollOffset).height()); 49 <tonikitoo> (sorry about the flood) 50 <andersca> tonikitoo: and this is inside blackberry specific code? 51 <tonikitoo> andersca, yes 52 <tonikitoo> I can subclass it, indeed. 53 <andersca> tonikitoo: just static_cast<ScrollAnimatorBlackberry*>(mainFrameView->scrollAnimator()) and then call disable on it 54 <tonikitoo> was just trying to avoid that :-) 55 <tonikitoo> andersca, sounds good. I can restrict it, sure 56 <tonikitoo> andersca, thanks!
Comment on attachment 138999 [details] (r115347, r=andersca) patch v3 View in context: https://bugs.webkit.org/attachment.cgi?id=138999&action=review > Source/WebCore/platform/ScrollAnimator.h:103 > + virtual void animationWillStart() { } > + virtual void animationDidFinish() { } I think you can just put these in ScrollAnimatorNone.
Comment on attachment 138999 [details] (r115347, r=andersca) patch v3 Committed: <http://trac.webkit.org/changeset/115347>