Bug 110183 – [chromium] Fix races in double-tap zoom minimum scale policy

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alexandre Elias]

[chromium] Fix races in double-tap zoom minimum scale policy

[Alexandre Elias]

Created attachment 188997 [details]
Patch

[Adam Barth]

Comment on attachment 188997 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=188997&action=review

This code really needs to be refactored.  All the page scaling state should be moved into a separate object that can be unit tested without introducing a bunch of "for testing" state.  One way to do that is to have the object call back through a virtual interface that actually operators on the WebView.  For testing, we can supply a mock implementation of the interface (e.g., so we don't need a "for testing" branch to avoid m_layerTreeView->startPageScaleAnimation).

We can land this patch as-is, but please consider a followup patch to refactor this code.

> Source/WebKit/chromium/src/WebViewImpl.cpp:406
> +    , m_doubleTapZoomPageScaleFactor(0)
> +    , m_doubleTapZoomPending(false)
> +    , m_enableFakeDoubleTapAnimationForTesting(false)
> +    , m_fakeDoubleTapPageScaleFactor(0)
> +    , m_fakeDoubleTapUseAnchor(false)

So much state!  We really should move all this viewport related state out into its own object.

[Alexandre Elias]

Comment on attachment 188997 [details]
Patch

Agreed completely, filed http://crbug.com/176998 to track.

[WebKit Review Bot]

Comment on attachment 188997 [details]
Patch

Clearing flags on attachment: 188997

Committed r143355: <http://trac.webkit.org/changeset/143355>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.