Bug 143199 - Swipe snapshot removed too early (jumps around) on arstechnica and NYT
Summary: Swipe snapshot removed too early (jumps around) on arstechnica and NYT
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-29 14:37 PDT by Tim Horton
Modified: 2015-03-30 12:04 PDT (History)
4 users (show)

See Also:

Attachments
Patch (18.32 KB, patch)
2015-03-29 14:37 PDT, Tim Horton 		no flags Details | Formatted Diff | Diff
Patch for landing (19.85 KB, patch)
2015-03-29 19:50 PDT, Tim Horton 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2015-03-29 14:37:21 PDT 
REGRESSION: Swipe snapshot removed too early (jumps around) on arstechnica and NYT
Comment 1 Tim Horton 2015-03-29 14:37:43 PDT 
Created attachment 249700 [details]
Patch
Comment 2 mitz 2015-03-29 15:48:12 PDT 
Comment on attachment 249700 [details]
Patch

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

> Source/WebKit2/ChangeLog:3
> +        REGRESSION: Swipe snapshot removed too early (jumps around) on arstechnica and NYT

Regression or not a regression?
Comment 3 Darin Adler 2015-03-29 15:57:17 PDT 
Comment on attachment 249700 [details]
Patch

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

> Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm:151
> +    , m_swipeWaitingForRenderTreeSizeThreshold(false)
> +    , m_swipeWaitingForRepaint(false)
> +    , m_swipeWaitingForDidFinishLoad(false)
> +    , m_swipeWaitingForSubresourceLoads(false)
> +    , m_swipeWaitingForScrollPositionRestoration(false)

Should initialize these in the header instead of the constructor. Should do the same for other scalars that are being initialized to an “obvious” default such as “false” or “0”. Helps keep changes in the different per-platform .mm files to a minimum and makes it harder to forget to initialize one.
Comment 4 Tim Horton 2015-03-29 16:27:47 PDT 
Not a regression in the WebKit sense. Will remove from the changelog (I realized after I uploaded the patch).
Comment 5 Tim Horton 2015-03-29 16:28:46 PDT 
(In reply to comment #3)
> Comment on attachment 249700 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=249700&action=review
> 
> > Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm:151
> > +    , m_swipeWaitingForRenderTreeSizeThreshold(false)
> > +    , m_swipeWaitingForRepaint(false)
> > +    , m_swipeWaitingForDidFinishLoad(false)
> > +    , m_swipeWaitingForSubresourceLoads(false)
> > +    , m_swipeWaitingForScrollPositionRestoration(false)
> 
> Should initialize these in the header instead of the constructor. Should do
> the same for other scalars that are being initialized to an “obvious”
> default such as “false” or “0”. Helps keep changes in the different
> per-platform .mm files to a minimum and makes it harder to forget to
> initialize one.

Yes! Will fix. Thanks!
Comment 6 Tim Horton 2015-03-29 19:50:31 PDT 
Created attachment 249706 [details]
Patch for landing
Comment 7 Tim Horton 2015-03-30 12:04:02 PDT 
http://trac.webkit.org/changeset/182150