913114 - Crash in nsBaseWidget::Release() on shutdown with apz

Status: RESOLVED FIXED

(Core Graveyard :: Widget: WinRT, defect)

Description

[Jim Mathies [:jimm]]

We'll need to track this down before turning apz on.

https://tbpl.mozilla.org/php/getParsedLog.php?id=27438761&tree=Try&full=1#error0

Comment 1

[Jim Mathies [:jimm]]

Attachment: stack.txt

Looks like the compositor thread is shutting down way too late.

Comment 2

[Jim Mathies [:jimm]]

CompositorParent definitely addrefs the widget, so maybe there is a ref counting bug somewhere besides gfx.

Comment 3

[Jim Mathies [:jimm]]

We're ending up with two ref counts here, GeckoContentController doesn't support nsISupports, but does implement its own inline addref/release. MetroWidget inherits addref, release and qi from nsBaseWidget.

Comment 4

[Jim Mathies [:jimm]]

Attachment: split GeckoContentController off from MetroWidget

Splitting these up so they can be torn down independently seemed like the simplest fix.

Comment 5

[Brian R. Bondy [:bbondy]]

Comment on attachment 800799 [details] [diff] [review]
split GeckoContentController off from MetroWidget

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

nit: Maybe we should just move APZController into its own .h and .cpp files.

::: widget/windows/winrt/MetroInput.cpp
@@ -160,5 @@
>    LogFunction();
>    NS_ASSERTION(aWidget, "Attempted to create MetroInput for null widget!");
>    NS_ASSERTION(aWindow, "Attempted to create MetroInput for null window!");
>  
> -  mWidget->SetMetroInput(this);

Are these still present in this file?
 // Used by MetroWidget GeckoContentController callbacks
  void HandleDoubleTap(const mozilla::LayoutDeviceIntPoint& aPoint);
  void HandleSingleTap(const mozilla::LayoutDeviceIntPoint& aPoint);
  void HandleLongTap(const mozilla::LayoutDeviceIntPoint& aPoint);

If so I think those are unused now and can also be removed?

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/471064addd39