1443187 - Crash in mozilla::layers::APZCTreeManager::NotifyLayerTreeAdopted

Status: RESOLVED FIXED

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

Description

[Marcia Knous [:marcia]]

This bug was filed from the Socorro interface and is
report bp-87d321dc-94eb-4051-a4a7-2fcfa0180304.
=============================================================

Seen while reviewing nightly crash data - crashes started using 20180303100406: http://bit.ly/2oIQmFo

Possible regression range based on Build ID: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b2a9a4bb5c94de179ae7a3f52fde58c0e2897498&tochange=8cced2a46f73238da13e41bcae8f6f801419bb7a

Bug 1441916 landed in that timeframe.

Top 10 frames of crashing thread:

0 xul.dll mozilla::layers::APZCTreeManager::NotifyLayerTreeAdopted gfx/layers/apz/src/APZCTreeManager.cpp:275
1 xul.dll mozilla::layers::CompositorBridgeParent::RecvAdoptChild gfx/layers/ipc/CompositorBridgeParent.cpp:1716
2 xul.dll mozilla::layers::PCompositorBridgeParent::OnMessageReceived ipc/ipdl/PCompositorBridgeParent.cpp:761
3 xul.dll mozilla::layers::PCompositorManagerParent::OnMessageReceived ipc/ipdl/PCompositorManagerParent.cpp:121
4 xul.dll mozilla::ipc::MessageChannel::DispatchAsyncMessage ipc/glue/MessageChannel.cpp:2133
5 xul.dll mozilla::ipc::MessageChannel::DispatchMessageW ipc/glue/MessageChannel.cpp:2063
6 xul.dll mozilla::ipc::MessageChannel::RunMessage ipc/glue/MessageChannel.cpp:1909
7 xul.dll mozilla::ipc::MessageChannel::MessageTask::Run ipc/glue/MessageChannel.cpp:1942
8 xul.dll MessageLoop::DoWork ipc/chromium/src/base/message_loop.cc:535
9 xul.dll base::MessagePumpForUI::DoRunLoop ipc/chromium/src/base/message_pump_win.cc:210

=============================================================

Comment 1

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

The crash seems to be because a layer tree is getting "adopted" but it doesn't have an oldApzSampler. The comment at [1] implies that it's possible for a document which has no presentation (and therefore no layer tree) to trigger an "adoption" message. In this case there would be no "old" APZSampler. So I guess we should handle that case more gracefully.

[1] https://searchfox.org/mozilla-central/rev/bffd3e0225b65943364be721881470590b9377c1/layout/ipc/RenderFrameParent.cpp#225

Comment 2

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=528d788c3add6c3b006bb7d2eaef678d8508f472

Comment 3

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

Attachment: Bug 1443187 - Avoid crashing if a previously-uncomposited layer tree gets adopted.

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

Comment 4

[Botond Ballo [:botond]]

Comment on attachment 8956464 [details]
Bug 1443187 - Avoid crashing if a previously-uncomposited layer tree gets adopted.

https://reviewboard.mozilla.org/r/225362/#review231784

::: gfx/layers/ipc/CompositorBridgeParent.cpp
(Diff revision 1)
>    }
>  
> -  // We don't support moving a child from a APZ-enabled compositor to a
> -  // APZ-disabled compostior. The mOptions assertion above should already
> -  // ensure this, since APZ-ness is one of the things in mOptions.
> -  MOZ_ASSERT((oldApzSampler != nullptr) == (mApzSampler != nullptr));

Can we keep in place a weaker version of this assertion, like:

  if (oldApzSampler) {
    MOZ_ASSERT(mApzSampler);
  }
  
?

Comment 5

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

(In reply to Botond Ballo [:botond] from comment #4)
> Can we keep in place a weaker version of this assertion, like:
> 
>   if (oldApzSampler) {
>     MOZ_ASSERT(mApzSampler);
>   }

Sure, that seems reasonable.

Comment 6

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

Comment on attachment 8956464 [details]
Bug 1443187 - Avoid crashing if a previously-uncomposited layer tree gets adopted.

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

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/020eaf331263
Avoid crashing if a previously-uncomposited layer tree gets adopted. r=botond

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/020eaf331263