1495169 - Page top/bottom part is truncated after change width of content area if you keep moving the mouse pointer over the content area

Status: VERIFIED FIXED

(Core :: Layout, defect, P3)

Description

[Alice0775 White]

Firefox62.0.2  : Can not reproduce with the following str
Firefox63.0b10 : Cat not reproduce with the following str
Nightly64.0a1  : Reproduce


Reproducible: almost always

Steps To Reproduce:

1. Browser width to be 1280px
2. Open Sidebar(ctrl+B)
3. Open https://www.ecma-international.org/ecma-262/8.0/index.html
4. Wait for page loading
5. Scroll to bottom
6. Wait to render
7. Close Sidebar(ctrl+B)
   --- Bug appears, now, page bottom part is truncated
8. Click on "G Copyright & Software License" at bottom of left side menu
   --- Page bottom part appears
9. Scroll to top
   --- Bug appears, now, page top part is truncated


Actual Results:
Page top and bottom part is truncated.
Unable to scroll to page top and bottom.


Expected Results:
It should not be truncated.
It can scroll to page top and bottom

Comment 1

[Alice0775 White]

Attachment: about:support

Comment 2

[Alice0775 White]

Attachment: screenshot after doing step 7 and step 9

Comment 3

[Hiroyuki Ikezoe (:hiro)]

I can see the issue on Linux. Setting P3 tentatively.

Comment 4

[Alice0775 White]

On 63.0b10, reproduce the issue with the following STR

STR
1. Browser width to be 1280px
2. Open Sidebar(ctrl+B)
3. Open https://www.ecma-international.org/ecma-262/8.0/index.html
4. Wait for page loading
5. Scroll to bottom
6. Wait to render
7. Close Sidebar(ctrl+B)
8. Scroll to bottom
   --- OK, you can see page bottom part as expected
9. Open Sidebar(ctrl+B)
10. Scroll to bottom
   --- Bug appears, now, page bottom part is truncated

11. if not reproduce, repeat from step 7

Comment 5

[Alice0775 White]

It may be necessary to repeat STR of comment#4 3-5 times on 63.0b3

Comment 6

[Alice0775 White]

It seems that a bug occurs if you keep moving the mouse pointer over the content area.

Reproduced: 60.2.1esr, 62.0.2, 63.0b10, Nightly64.0a1


STR
1. Browser width to be 1280px
2. Open Sidebar(ctrl+B)
3. Open https://www.ecma-international.org/ecma-262/8.0/index.html
4. Wait for page loading
5. Scroll to bottom
6. Wait for render

7. Close Sidebar(ctrl+B)
8. Scroll to bottom
9. Open Sidebar(ctrl+B)
10. you keep moving the mouse pointer over the content area until redrawing is completed
11. Scroll to bottom
   --- Bug appears, now, page bottom part is truncated

11. if not reproduce, repeat from step 7



If you set layout.interruptible-reflow.enabled to false, the problem does not seem to happen.

Comment 7

[Alice0775 White]

e10s is not matter. I can reproduce the issue with and without e10s both.

Comment 8

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=bdc513580a45e6392816a8de4f3da0c8da5e72b0&tochange=b4730a967e2c796fba51576da7d9e91c4ae557c8

Regressed by: Bug 1209697

Comment 9

[Boris Zbarsky [:bzbarsky]]

So as long as you keep moving the mouse we will interrupt reflows; that part is expected.  But you're saying that you stop moving the mouse and the rendering still does not update?

Comment 10

[Alice0775 White]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #9)
> So as long as you keep moving the mouse we will interrupt reflows; that part
> is expected.  But you're saying that you stop moving the mouse and the
> rendering still does not update?

Yes I can reproduce the issue on Nightky64.0a1 without mouse moving.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Daniel, do you have time to take a look or know someone on layout who does?

Comment 12

[Emilio Cobos Álvarez (:emilio)]

This looks very similar to other bug that was reported recently (can't find it r/n). My theory is that a measuring reflow for a flex item gets interrupted and we incorrectly cache the measurement there, then we incorrectly use it for other non-interruptible reflows.

Boris, is checking HasPendingInterrupt at the beginning of reflow Reflow() and in DidReflow() enough to tell whether any of our kids was interrupted?

If so, we should probably just clear the cached measuring reflow in that case to avoid using it in subsequent reflows.

Comment 13

[Boris Zbarsky [:bzbarsky]]

> is checking HasPendingInterrupt at the beginning of reflow Reflow() and in DidReflow() enough to tell whether any of our kids was interrupted?

I think so, yes.

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1495169 - Remove cached measuring reflows if we aren't measuring the right thing because we got interrupted.

If we're waiting on an interrupt, then our child items haven't been totally
reflowed and our measures would be bogus.

This will probably regress performance in the cases bug 1209697 fixed, so we
should probably add an interrupt check somewhere in nsFlexContainerFrame to
avoid keeping reflowing flex containers indefinitely.

We could probably just bail out from our reflow if any kid reflow was
interrupted.

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Alice can you check whether one of the builds from https://treeherder.mozilla.org/#/jobs?repo=try&revision=66f4b14d9a89f6068e492ec490dfd7e500a635ad fixes the issue? Thanks!

Comment 16

[Boris Zbarsky [:bzbarsky]]

> We could probably just bail out from our reflow if any kid reflow was interrupted.

If we can do that, it would be ideal.

Comment 17

[Emilio Cobos Álvarez (:emilio)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #15)
> Alice can you check whether one of the builds from
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=66f4b14d9a89f6068e492ec490dfd7e500a635ad fixes the
> issue? Thanks!

Actually, https://treeherder.mozilla.org/#/jobs?repo=try&revision=5f4068771c55cd909ce5f2806bca73cfa79ffc48, since the build above is probably slightly broken (forgot to call the base class method, sigh).

Comment 18

[Alice0775 White]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #15)
> Alice can you check whether one of the builds from
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=66f4b14d9a89f6068e492ec490dfd7e500a635ad fixes the
> issue? Thanks!


The try build fixes the issue!

Comment 19

[Daniel Holbert [:dholbert]]

[Canceling ni & assigning to Emilio, since it looks like he's got this (thanks Emilio!)]

Comment 20

[Alice0775 White]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #17)
> Actually,
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=5f4068771c55cd909ce5f2806bca73cfa79ffc48, since the
> build above is probably slightly broken (forgot to call the base class
> method, sigh).

The try build(x64 pgo, x86 opt) also fixes this issue.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6daf79d1bace
Remove cached measuring reflows if we aren't measuring the right thing because we got interrupted. r=dholbert

Comment 22

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/6daf79d1bace

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Is this something we should consider for Beta backport with the RCs being next week?

Comment 24

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9013310 [details]
Bug 1495169 - Remove cached measuring reflows if we aren't measuring the right thing because we got interrupted.

It's safe I'd say, it's only clearing cached stuff.

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1209697

User impact if declined: Some big pages may be wrongly sized if interacted with at the wrong time.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: Yes

If yes, steps to reproduce: See comment 0, or bug 1362758.

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Only clearing cached stuff when we get interrupted, so should have ~0 risk.

String changes made/needed: none

Comment 25

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9013310 [details]
Bug 1495169 - Remove cached measuring reflows if we aren't measuring the right thing because we got interrupted.

Minimal patch fixing a 63 regression, evaluated to very low risk, uplift approved for our last 63 beta, thanks.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/9f144c575382

Comment 27

[Anca Soncutean, Desktop QA]

I’ve reproduce this issue on Fx 63.0b10 (20180927143327) with Windows 10 x64, by following the steps provided in comment 6.
On Fx 63.0b14 (20181011200118) and Fx 64.0a1 (2018-10-11), using Windows 10 x64, Ubuntu 16.04 x86 and macOS 10.13, the issue is no longer reproducible.