943164 - TabChild should not assume that APZ being turned on implies there is a root APZC

Status: RESOLVED INVALID

(Core :: Panning and Zooming, defect)

Description

[Botond Ballo [:botond]]

TabChild appears to assume that if APZ is turned on (TabChild::IsAsyncPanZoomEnabled()), then there will be at least one (root) APZC.

For example, in TabChild::Observe(), the code that handles "before-first-paint" sets the viewport and resolution to some default values, expecting that the following call to HandlePossibleViewportChange() will set them to correct values. Both pieces of code only execute if APZ is enabled; the code in HandlePossibleViewportChange() only executes if there is also a root APZC.

With the patch for bug 942995, we can now have APZ enabled without having a root APZC, which means the resolution will be set to the (undesirable) default values, and then never set to the correct values.

Comment 1

[Botond Ballo [:botond]]

Attachment: Part 1 - Do not exit early from TabChild::HandlePossibleViewportChange() if there is no root APZC

Comment 2

[Botond Ballo [:botond]]

Try run for this and bug 942995 together: https://tbpl.mozilla.org/?tree=Try&rev=1089ad70b773

Comment 3

[Botond Ballo [:botond]]

Attachment: Part 2 - Do not increase CSS viewport height beyond screen height if there is no root APZC

Comment 4

[Botond Ballo [:botond]]

Comment on attachment 8338796 [details] [diff] [review]
Part 2 - Do not increase CSS viewport height beyond screen height if there is no root APZC

Whoops, this is not quite finished.

Comment 5

[Botond Ballo [:botond]]

Attachment: Part 2 - Do not increase CSS viewport height beyond screen height if there is no root APZC

Comment 6

[Benoit Girard (:BenWa)]

Comment on attachment 8338809 [details] [diff] [review]
Part 2 - Do not increase CSS viewport height beyond screen height if there is no root APZC

Review of attachment 8338809 [details] [diff] [review]:
-----------------------------------------------------------------

IMO I'm not convinced this is the right place at all for us to have logic that changes the viewport like this. But you're not making it worse.

Comment 7

[Benoit Girard (:BenWa)]

To be clear, I was referring to the original code you modified, not your changes.

Comment 8

[Botond Ballo [:botond]]

After further discussion (see bug 940691), we do always want to keep a root APZC when APZ is enabled. Resolving as invalid.