1458259 - Remove invalid assertion in UpdateZoomConstraints

Status: RESOLVED FIXED

(Core :: Panning and Zooming, enhancement, P3)

Description

[Kartikaya Gupta (email:[e-mail])]

Fix for https://bugzilla.mozilla.org/show_bug.cgi?id=1449982#c75 - the assertion in UpdateZoomConstraints is no longer accurate, since with GPU process and async-scene-building enabled in WR, it can get called on the compositor thread in the GPU process, which is not the updater thread.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Bas was hitting this assertion failure in his local debug windows build with WR enabled.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1458259 - Remove assertion that is no longer valid.

Review commit: https://reviewboard.mozilla.org/r/240972/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/240972/

Comment 3

[Botond Ballo [:botond]]

Comment on attachment 8972347 [details]
Bug 1458259 - Remove assertion that is no longer valid.

https://reviewboard.mozilla.org/r/240972/#review247028

Comment 4

[Botond Ballo [:botond]]

Comment on attachment 8972347 [details]
Bug 1458259 - Remove assertion that is no longer valid.

https://reviewboard.mozilla.org/r/240972/#review247034

::: gfx/layers/apz/src/APZCTreeManager.cpp:1989
(Diff revision 1)
>  {
>    if (!GetUpdater()->IsUpdaterThread()) {
>      // This can happen if we're in the UI process and got a call directly from
> -    // nsBaseWidget (as opposed to over PAPZCTreeManager). We want this function
> -    // to run on the updater thread, so bounce it over.
> -    MOZ_ASSERT(XRE_IsParentProcess());
> +    // nsBaseWidget (as opposed to over PAPZCTreeManager). It can also happen
> +    // in the GPU process if that is enabled, since the call will go over
> +    // PAPZCTreeManager and arrive on the compositor thread in the GPU process.

If the GPU process is disabled but WR + async scene building are enabled, this can also happen on the UI process compositor thread receiving a message over PAPZCTreeManager, can't it?

(That is, the removal of the assertion is fine, but the comment is still misleading.)

Comment 5

[Kartikaya Gupta (email:[e-mail])]

(In reply to Botond Ballo [:botond] from comment #4)
> If the GPU process is disabled but WR + async scene building are enabled,
> this can also happen on the UI process compositor thread receiving a message
> over PAPZCTreeManager, can't it?

Yup, you're right. I'll update the comment.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8972347 [details]
Bug 1458259 - Remove assertion that is no longer valid.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/240972/diff/1-2/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/38b1ed4d8bd2
Remove assertion that is no longer valid. r=botond

Comment 8

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/38b1ed4d8bd2