Bug 216368

Summary:

REGRESSION (r255383): Transition from email to password field on login.live.com stutters after going back and forth

Product:

WebKit

Reporter:

Antoine Quint <graouts>

Component:

Compositing

Assignee:

Antoine Quint <graouts>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

changseok, esprehn+autocc, ews-watchlist, fred.wang, glenn, graouts, kondapallykalyan, pdr, simon.fraser, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Reduction	none
Patch for EWS	none
WIP	none
Patch	none
Patch	none
Patch	none

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].