Bug 669105 - Leak-until-shutdown with deviceorientation and unload listeners. r=smaug
authorDoug Turner <dougt@dougt.org>
Wed, 06 Jul 2011 22:56:32 -0700
changeset 72658 d734e168a228fa727280a727f83f77fb7121e63e
parent 72657 c8afe7ab83e76014fcb08f8315380e9ff97fed46
child 72659 25812390573a075bf38cc81ee3031be7c127e94b
push idunknown
push userunknown
push dateunknown
reviewerssmaug
bugs669105
milestone8.0a1
Bug 669105 - Leak-until-shutdown with deviceorientation and unload listeners. r=smaug
dom/base/nsGlobalWindow.cpp
dom/system/nsDeviceMotion.cpp
dom/system/nsDeviceMotion.h
dom/tests/mochitest/Makefile.in
dom/tests/mochitest/orientation/Makefile.in
dom/tests/mochitest/orientation/test_bug507902.html
xpcom/system/nsIDeviceMotion.idl
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -1021,16 +1021,18 @@ nsGlobalWindow::~nsGlobalWindow()
 
   mDocument = nsnull;           // Forces Release
   mDoc = nsnull;
 
   NS_ASSERTION(!mArguments, "mArguments wasn't cleaned up properly!");
 
   CleanUp(PR_TRUE);
 
+  NS_ASSERTION(!mHasDeviceMotion, "Window still registered with device motion.");
+
 #ifdef DEBUG
   nsCycleCollector_DEBUG_wasFreed(static_cast<nsIScriptGlobalObject*>(this));
 #endif
 
   if (mURLProperty) {
     mURLProperty->ClearWindowReference();
   }
 
--- a/dom/system/nsDeviceMotion.cpp
+++ b/dom/system/nsDeviceMotion.cpp
@@ -158,17 +158,17 @@ nsDeviceMotion::TimeoutHandler(nsITimer 
   // usage.
   nsDeviceMotion *self = reinterpret_cast<nsDeviceMotion *>(aClosure);
   if (!self) {
     NS_ERROR("no self");
     return;
   }
   
   // what about listeners that don't clean up properly?  they will leak
-  if (self->mListeners.Count() == 0 && self->mWindowListeners.Count() == 0) {
+  if (self->mListeners.Count() == 0 && self->mWindowListeners.Length() == 0) {
     self->Shutdown();
     self->mStarted = PR_FALSE;
   }
 }
 
 NS_IMETHODIMP nsDeviceMotion::AddListener(nsIDeviceMotionListener *aListener)
 {
   if (mListeners.IndexOf(aListener) >= 0)
@@ -190,52 +190,44 @@ NS_IMETHODIMP nsDeviceMotion::RemoveList
 
   mListeners.RemoveObject(aListener);
   StartDisconnectTimer();
   return NS_OK;
 }
 
 NS_IMETHODIMP nsDeviceMotion::AddWindowListener(nsIDOMWindow *aWindow)
 {
-  if (mWindowListeners.IndexOf(aWindow) >= 0)
-    return NS_OK; // already exists
-
   if (mStarted == PR_FALSE) {
     mStarted = PR_TRUE;
     Startup();
   }
-
-  mWindowListeners.AppendObject(aWindow);
+  mWindowListeners.AppendElement(aWindow);
   return NS_OK;
 }
 
 NS_IMETHODIMP nsDeviceMotion::RemoveWindowListener(nsIDOMWindow *aWindow)
 {
-  if (mWindowListeners.IndexOf(aWindow) < 0)
-    return NS_OK; // doesn't exist
-
-  mWindowListeners.RemoveObject(aWindow);
+  mWindowListeners.RemoveElement(aWindow);
   StartDisconnectTimer();
-
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDeviceMotion::DeviceMotionChanged(PRUint32 type, double x, double y, double z)
 {
   if (!mEnabled)
     return NS_ERROR_NOT_INITIALIZED;
 
   for (PRUint32 i = mListeners.Count(); i > 0 ; ) {
     --i;
     nsRefPtr<nsDeviceMotionData> a = new nsDeviceMotionData(type, x, y, z);
     mListeners[i]->OnMotionChange(a);
   }
 
-  for (PRUint32 i = mWindowListeners.Count(); i > 0 ; ) {
+  for (PRUint32 i = mWindowListeners.Length(); i > 0 ; ) {
     --i;
 
     nsCOMPtr<nsIDOMDocument> domdoc;
     mWindowListeners[i]->GetDocument(getter_AddRefs(domdoc));
 
     if (domdoc) {
       nsCOMPtr<nsIDOMEventTarget> target = do_QueryInterface(mWindowListeners[i]);
       if (type == nsIDeviceMotionData::TYPE_ACCELERATION)
--- a/dom/system/nsDeviceMotion.h
+++ b/dom/system/nsDeviceMotion.h
@@ -35,16 +35,17 @@
  * ***** END LICENSE BLOCK ***** */
 
 #ifndef nsDeviceMotion_h
 #define nsDeviceMotion_h
 
 #include "nsIDeviceMotion.h"
 #include "nsIDOMDeviceMotionEvent.h"
 #include "nsCOMArray.h"
+#include "nsTPtrArray.h"
 #include "nsCOMPtr.h"
 #include "nsITimer.h"
 
 #define NS_DEVICE_MOTION_CID \
 { 0xecba5203, 0x77da, 0x465a, \
 { 0x86, 0x5e, 0x78, 0xb7, 0xaf, 0x10, 0xd8, 0xf7 } }
 
 #define NS_DEVICE_MOTION_CONTRACTID "@mozilla.org/devicemotion;1"
@@ -59,17 +60,17 @@ public:
   NS_DECL_NSIDEVICEMOTIONUPDATE
 
   nsDeviceMotion();
 
   virtual ~nsDeviceMotion();
 
 private:
   nsCOMArray<nsIDeviceMotionListener> mListeners;
-  nsCOMArray<nsIDOMWindow> mWindowListeners;
+  nsTPtrArray<nsIDOMWindow> mWindowListeners;
 
   void StartDisconnectTimer();
 
   PRBool mStarted;
 
   nsCOMPtr<nsITimer> mTimeoutTimer;
   static void TimeoutHandler(nsITimer *aTimer, void *aClosure);
 
--- a/dom/tests/mochitest/Makefile.in
+++ b/dom/tests/mochitest/Makefile.in
@@ -50,16 +50,17 @@ DIRS	+= \
 	ajax \
 	bugs \
 	chrome \
 	general \
 	whatwg \
 	geolocation \
 	globalstorage \
 	localstorage \
+	orientation \
 	sessionstorage \
 	storageevent \
 	$(NULL)
 
 #needs IPC support, also tests do not run successfully in Firefox for now
 #ifneq (mobile,$(MOZ_BUILD_APP))
 #DIRS	+= notification
 #endif
new file mode 100644
--- /dev/null
+++ b/dom/tests/mochitest/orientation/Makefile.in
@@ -0,0 +1,53 @@
+# ***** BEGIN LICENSE BLOCK *****
+# Version: MPL 1.1/GPL 2.0/LGPL 2.1
+#
+# The contents of this file are subject to the Mozilla Public License Version
+# 1.1 (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+# http://www.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS IS" basis,
+# WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+# for the specific language governing rights and limitations under the
+# License.
+#
+# The Original Code is mozilla.org build system.
+#
+# The Initial Developer of the Original Code is Mozilla Foundation
+# Portions created by the Initial Developer are Copyright (C) 2011
+# the Initial Developer. All Rights Reserved.
+#
+# Contributor(s):
+#  Doug Turner <dougt@dougt.org>
+#
+# Alternatively, the contents of this file may be used under the terms of
+# either the GNU General Public License Version 2 or later (the "GPL"), or
+# the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+# in which case the provisions of the GPL or the LGPL are applicable instead
+# of those above. If you wish to allow use of your version of this file only
+# under the terms of either the GPL or the LGPL, and not to allow others to
+# use your version of this file under the terms of the MPL, indicate your
+# decision by deleting the provisions above and replace them with the notice
+# and other provisions required by the GPL or the LGPL. If you do not delete
+# the provisions above, a recipient may use your version of this file under
+# the terms of any one of the MPL, the GPL or the LGPL.
+#
+# ***** END LICENSE BLOCK *****
+
+DEPTH		= ../../../..
+topsrcdir	= @top_srcdir@
+srcdir		= @srcdir@
+VPATH		= @srcdir@
+relativesrcdir	= dom/tests/mochitest/orientation
+
+include $(DEPTH)/config/autoconf.mk
+
+include $(topsrcdir)/config/rules.mk
+
+_TEST_FILES	= \
+		test_bug507902.html \
+		$(NULL)
+
+libs:: 	$(_TEST_FILES)
+	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
+
new file mode 100644
--- /dev/null
+++ b/dom/tests/mochitest/orientation/test_bug507902.html
@@ -0,0 +1,39 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=507902
+-->
+<head>
+  <title>Test for watchPosition </title>
+  <script type="text/javascript" src="/MochiKit/packed.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=507902">Mozilla Bug 507902</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+function boom()
+{
+  window.addEventListener("unload", function(){}, false);
+  window.addEventListener("devicemotion", function(){}, false);
+  location = "data:text/html,2";
+
+  ok(1, "leak will be at the end of mochitests. so pass.");
+  SimpleTest.finish();
+}
+
+SimpleTest.waitForExplicitFinish();
+
+window.addEventListener("load", function() { setTimeout(boom, 0); }, false);
+
+</script>
+</pre>
+</body>
+</html>
+
--- a/xpcom/system/nsIDeviceMotion.idl
+++ b/xpcom/system/nsIDeviceMotion.idl
@@ -52,24 +52,26 @@ interface nsIDeviceMotionData : nsISuppo
 };
 
 [scriptable, uuid(f01774a2-3b7e-4630-954b-196dc178221f)]
 interface nsIDeviceMotionListener : nsISupports
 {
   void onMotionChange(in nsIDeviceMotionData aMotionData);
 };
 
-[scriptable, uuid(4B04E228-0B33-43FC-971F-AF60CEDB1C21)]
+[scriptable, uuid(B6E5C463-AAA6-44E2-BD07-7A7DC6192E68)]
 interface nsIDeviceMotion : nsISupports
 {
   void addListener(in nsIDeviceMotionListener aListener);
   void removeListener(in nsIDeviceMotionListener aListener);
 
-  void addWindowListener(in nsIDOMWindow aWindow);
-  void removeWindowListener(in nsIDOMWindow aWindow);
+  // Holds pointers, not AddRef objects -- it is up to the caller
+  // to call RemoveWindowListener before the window is deleted.
+  [noscript] void addWindowListener(in nsIDOMWindow aWindow);
+  [noscript] void removeWindowListener(in nsIDOMWindow aWindow);
 
 };
 
 /* for use by IPC system to notify non-chrome processes of 
  * device motion events
  */
 [uuid(d3a56f08-b7b1-46bb-9dc1-fc3665a3631a)]
 interface nsIDeviceMotionUpdate : nsIDeviceMotion