Bug 216368 - REGRESSION (r255383): Transition from email to password field on login.live.com stutters after going back and forth
Summary: REGRESSION (r255383): Transition from email to password field on login.live.c...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Compositing (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-09-10 08:23 PDT by Antoine Quint
Modified: 2020-09-11 22:54 PDT (History)
10 users (show)

See Also:


Attachments
Reduction (1.96 KB, text/html)
2020-09-10 08:25 PDT, Antoine Quint
no flags Details
Patch for EWS (1.37 KB, patch)
2020-09-11 06:25 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
WIP (1.65 KB, patch)
2020-09-11 06:54 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch (6.41 KB, patch)
2020-09-11 07:18 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch (6.47 KB, patch)
2020-09-11 12:01 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch (6.50 KB, patch)
2020-09-11 12:04 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2020-09-10 08:23:56 PDT
When accessing login.live.com and inputting an email, there is a transition to present the next screen for inputting a password. If the user clicks the back arrow to return to the email input, the animation of that transition stutters. This will occur every time the use transitions forwards or backwards in this login flow.

Steps to Reproduce:
1. Launch Safari
2. Access login.live.com
3. Input an email
4. Click “Next”, verifying the smooth transition
5. Click the back arrow next to the email address 

Expected Results: Transition back to the user name field should be smooth

Actual Results: Transition back to the user name field is jittery, causing the text to stutter
Comment 1 Antoine Quint 2020-09-10 08:24:20 PDT
<rdar://problem/67019460>
Comment 2 Antoine Quint 2020-09-10 08:25:07 PDT
Created attachment 408447 [details]
Reduction
Comment 3 Antoine Quint 2020-09-11 06:25:29 PDT
Created attachment 408528 [details]
Patch for EWS
Comment 4 Antoine Quint 2020-09-11 06:47:56 PDT
Trying to understand the code here. In RenderLayerCompositor::layerStyleChanged(), I see this comment:

    // Create or destroy backing here so that code that runs during layout can reliably use isComposited() (though this
    // is only true for layers composited for direct reasons).
    // Also, it allows us to avoid a tree walk in updateCompositingLayers() when no layer changed its compositing state.

This makes me believe that when we call updateBacking() and that it returns true as the second animation starts (and we have the duplicate fader), the layout isn't up-to-date and so calling repaintOnCompositingChange() under updateBacking() happens before we can paint with the right layout.

Calling repaintOnCompositingChange() in a further updateBacking() call for the same layer certainly fixes the issue, but then it's probably called too often and I doubt it's legitimate.
Comment 5 Antoine Quint 2020-09-11 06:54:04 PDT
Created attachment 408529 [details]
WIP
Comment 6 Antoine Quint 2020-09-11 07:18:22 PDT
Created attachment 408531 [details]
Patch
Comment 7 Simon Fraser (smfr) 2020-09-11 08:37:48 PDT
Comment on attachment 408531 [details]
Patch

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

> Source/WebCore/rendering/RenderLayerCompositor.cpp:958
> +        repaintOnCompositingChange(layer);

This isn't the right place to do this. The bug is about backing sharing. This is not a backing-sharing-specific change.
Comment 8 Antoine Quint 2020-09-11 12:01:03 PDT
Created attachment 408552 [details]
Patch
Comment 9 Antoine Quint 2020-09-11 12:04:46 PDT
Created attachment 408553 [details]
Patch
Comment 10 EWS 2020-09-11 22:54:23 PDT
Committed r266972: <https://trac.webkit.org/changeset/266972>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 408553 [details].