1030126 - APZCTM constructor might be getting called multiple times in some cases

Status: RESOLVED WORKSFORME

(Core :: Panning and Zooming, defect)

Description

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

Attachment: Try log extract

Based on the info at https://bugzilla.mozilla.org/show_bug.cgi?id=1030115#c2 it appears that there might be cases where we have multiple APZCTM instances created in a single process. This is unexpected (to me at least) and should be investigated.

The stack from the try push where the assertion happened is attached. Gijs suggested that it might be happening in a case where a new dialog or window is being created, which makes sense. I think in such a case it might even be ok to create multiple APZCTM instances (one per compositor), as long as the rest of our code deals with that properly.

Comment 1

[Markus Stange [:mstange]]

It's surprising to me that having multiple APZCTM instances is surprising :)

An APZCTM instance is created whenever a Compositor instance is created, so the invariant of having one APZCTM per compositor should be true. IIRC I copied the APZCTM creation code from the metro widget implementation, and I think it's possible to have multiple windows (each with its own compositor) on Metro.

Comment 2

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

I suppose it's surprising to me because we've primarily been focused on B2G where it doesn't happen, and never ran into a case on metro where it does happen.

I don't know if there needs be stuff in the APZCTM destructor to do cleanup as well then.

Comment 3

[Markus Stange [:mstange]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #2)
> I don't know if there needs be stuff in the APZCTM destructor to do cleanup
> as well then.

I think now that the pref cache is taken care off there's nothing left to do here. The constructor doesn't do anything other than calling AsyncPanZoomController::InitializeGlobalState() which already ignores subsequent calls, and conditioning mApzcTreeLog. I've also looked through the member variables of APZCTreeManager and haven't found anything that's not taken care of by their destructors. And any leaks should be caught by the gtests, I hope.

Comment 4

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

Thanks for looking into it! Closing this out then.