Bug 132115

Summary: WebKit2 View Gestures (Swipe): Discard snapshots made with a different view size/pixel density
Product: WebKit Reporter: Tim Horton <thorton>
Component: WebKit2Assignee: Tim Horton <thorton>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, mitz, sam, simon.fraser
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
none
fix the build
none
fix the ML build
none
pulled crash fix into another bug simon.fraser: review+

Description Tim Horton 2014-04-24 01:36:20 PDT
We shouldn't even bother trying to use swipe snapshots taken at a different view size or device pixel ratio, they always look terrible.

<rdar://problem/16681791>
Comment 1 Tim Horton 2014-04-26 02:52:53 PDT
*** Bug 132116 has been marked as a duplicate of this bug. ***
Comment 2 Tim Horton 2014-04-26 03:06:54 PDT
Created attachment 230239 [details]
patch
Comment 3 Tim Horton 2014-04-26 10:46:51 PDT
Created attachment 230244 [details]
fix the build
Comment 4 Tim Horton 2014-04-26 15:38:25 PDT
Comment on attachment 230244 [details]
fix the build

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

> Source/WebKit2/UIProcess/mac/ViewSnapshotStore.mm:154
>      String oldSnapshotUUID = item->snapshotUUID();
> -    if (!oldSnapshotUUID.isEmpty())
> -        m_snapshotMap.remove(oldSnapshotUUID);
> +    if (!oldSnapshotUUID.isEmpty()) {
> +        const auto& oldSnapshotIter = m_snapshotMap.find(oldSnapshotUUID);
> +        if (oldSnapshotIter != m_snapshotMap.end()) {
> +            if (oldSnapshotIter->value.hasImage())
> +                m_snapshotsWithImagesCount--;
> +            m_snapshotMap.remove(oldSnapshotIter);
> +        }
> +    }
>  

Pulled this bit out into https://bugs.webkit.org/show_bug.cgi?id=132204
Comment 5 Tim Horton 2014-04-26 19:02:27 PDT
Created attachment 230250 [details]
fix the ML build
Comment 6 Tim Horton 2014-04-26 19:03:36 PDT
Created attachment 230251 [details]
pulled crash fix into another bug
Comment 7 Tim Horton 2014-04-28 10:30:17 PDT
Comment on attachment 230251 [details]
pulled crash fix into another bug

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

> Source/WebKit2/ChangeLog:65
> +        Fix a bug where the count of snapshots with live images was too high
> +        because we were failing to decrement it when replacing a snapshot of
> +        an existing item with a fresh one.

get rid of the changelog part of this too (code change moved to a different bug)
Comment 8 Simon Fraser (smfr) 2014-04-28 10:48:48 PDT
Comment on attachment 230251 [details]
pulled crash fix into another bug

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

> Source/WebKit2/ChangeLog:9
> +        To do this, we need an accurate picture of the topContentInset, both for the Web view,

"picture" in the context of snapshotting is ... confusing.

> Source/WebKit2/UIProcess/mac/ViewGestureController.h:137
> +    CALayer *determineSnapshotLayerParent();
> +    CALayer *determineLayerAdjacentToSnapshotForParent(SwipeDirection, CALayer *snapshotLayerParent);

Not sure why these aren't const.
Comment 9 Tim Horton 2014-04-28 14:07:59 PDT
http://trac.webkit.org/changeset/167905