Closed Bug 913114 Opened 7 years ago Closed 7 years ago

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

Categories

(Core Graveyard :: Widget: WinRT, defect)

26 Branch
x86_64
Windows 8.1
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla26

People

(Reporter: jimm, Assigned: jimm)

References

Details

Attachments

(2 files)

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
Summary: Crash in nsBaseWidget::Release() [nsBaseWidget.cpp] on shutdown → Crash in nsBaseWidget::Release() on shutdown with apz
Attached file stack.txt
Looks like the compositor thread is shutting down way too late.
Component: Widget: WinRT → Graphics: Layers
CompositorParent definitely addrefs the widget, so maybe there is a ref counting bug somewhere besides gfx.
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.
Component: Graphics: Layers → Widget: WinRT
Splitting these up so they can be torn down independently seemed like the simplest fix.
Attachment #800799 - Flags: review?(netzen)
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?
Attachment #800799 - Flags: review?(netzen) → review+
https://hg.mozilla.org/mozilla-central/rev/471064addd39
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
OS: Windows 8 Metro → Windows 8.1
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.