Bug 110183

Summary: [chromium] Fix races in double-tap zoom minimum scale policy
Product: WebKit Reporter: Alexandre Elias <aelias>
Component: New BugsAssignee: Alexandre Elias <aelias>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, aelias, klobag, webkit.review.bot, yusufo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Alexandre Elias 2013-02-18 23:04:42 PST
[chromium] Fix races in double-tap zoom minimum scale policy
Comment 1 Alexandre Elias 2013-02-18 23:10:11 PST
Created attachment 188997 [details]
Patch
Comment 2 Adam Barth 2013-02-19 10:10:20 PST
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.
Comment 3 Alexandre Elias 2013-02-19 10:39:48 PST
Comment on attachment 188997 [details]
Patch

Agreed completely, filed http://crbug.com/176998 to track.
Comment 4 WebKit Review Bot 2013-02-19 10:44:10 PST
Comment on attachment 188997 [details]
Patch

Clearing flags on attachment: 188997

Committed r143355: <http://trac.webkit.org/changeset/143355>
Comment 5 WebKit Review Bot 2013-02-19 10:44:13 PST
All reviewed patches have been landed.  Closing bug.