patch.diff

# HG changeset patch
# User Kyle Huey <khuey@kylehuey.com>
# Date 1438350567 25200
# Node ID 2b518fc1e8a49e7f853ea92ebeecba00aa9f6a04
# Parent  96bb759922a1cdf2b9ce3642ec6de84c80ae0741
Bug 1152574: Fix console service discarding to run when it is actually safe to do so (off of inner-window-destroyed).  Also add a backstop at xpcom-shutdown to discard any cycle collected messages before it is too late. r=froydnj

diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -37,17 +37,16 @@
 #include "nsIScriptTimeoutHandler.h"
 #include "nsIController.h"
 #include "nsScriptNameSpaceManager.h"
 #include "nsISlowScriptDebug.h"
 #include "nsWindowMemoryReporter.h"
 #include "WindowNamedPropertiesHandler.h"
 #include "nsFrameSelection.h"
 #include "nsNetUtil.h"
-#include "nsIConsoleService.h"
 
 // Helper Classes
 #include "nsJSUtils.h"
 #include "jsapi.h"              // for JSAutoRequest
 #include "jswrapper.h"
 #include "nsReadableUtils.h"
 #include "nsDOMClassInfo.h"
 #include "nsJSEnvironment.h"
@@ -1562,22 +1561,16 @@ nsGlobalWindow::ClearControllers()
   }
 }
 
 void
 nsGlobalWindow::FreeInnerObjects()
 {
   NS_ASSERTION(IsInnerWindow(), "Don't free inner objects on an outer window");
 
-  // Prune messages related to this window in the console cache
-  nsCOMPtr<nsIConsoleService> console(do_GetService("@mozilla.org/consoleservice;1"));
-  if (console) {
-    console->ClearMessagesForWindowID(mWindowID);
-  }
-
   // Make sure that this is called before we null out the document and
   // other members that the window destroyed observers could
   // re-create.
   NotifyDOMWindowDestroyed(this);
 
   mInnerObjectsFreed = true;
 
   // Kill all of the workers for this window.
diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -4299,23 +4299,24 @@ ContentParent::HandleEvent(nsIDOMGeoPosi
 
 nsConsoleService *
 ContentParent::GetConsoleService()
 {
     if (mConsoleService) {
         return mConsoleService.get();
     }
 
+    // XXXkhuey everything about this is terrible.
     // Get the ConsoleService by CID rather than ContractID, so that we
     // can cast the returned pointer to an nsConsoleService (rather than
     // just an nsIConsoleService). This allows us to call the non-idl function
     // nsConsoleService::LogMessageWithMode.
     NS_DEFINE_CID(consoleServiceCID, NS_CONSOLESERVICE_CID);
-    nsCOMPtr<nsConsoleService>  consoleService(do_GetService(consoleServiceCID));
-    mConsoleService = consoleService;
+    nsCOMPtr<nsIConsoleService> consoleService(do_GetService(consoleServiceCID));
+    mConsoleService = static_cast<nsConsoleService*>(consoleService.get());
     return mConsoleService.get();
 }
 
 bool
 ContentParent::RecvConsoleMessage(const nsString& aMessage)
 {
     nsRefPtr<nsConsoleService> consoleService = GetConsoleService();
     if (!consoleService) {
diff --git a/xpcom/base/nsConsoleService.cpp b/xpcom/base/nsConsoleService.cpp
--- a/xpcom/base/nsConsoleService.cpp
+++ b/xpcom/base/nsConsoleService.cpp
@@ -17,16 +17,17 @@
 
 #include "nsConsoleService.h"
 #include "nsConsoleMessage.h"
 #include "nsIClassInfoImpl.h"
 #include "nsIConsoleListener.h"
 #include "nsPrintfCString.h"
 #include "nsProxyRelease.h"
 #include "nsIScriptError.h"
+#include "nsISupportsPrimitives.h"
 
 #include "mozilla/Preferences.h"
 
 #if defined(ANDROID)
 #include <android/log.h>
 #include "mozilla/dom/ContentChild.h"
 #endif
 #ifdef XP_WIN
@@ -40,18 +41,18 @@ using namespace mozilla::tasktracer;
 
 using namespace mozilla;
 
 NS_IMPL_ADDREF(nsConsoleService)
 NS_IMPL_RELEASE(nsConsoleService)
 NS_IMPL_CLASSINFO(nsConsoleService, nullptr,
                   nsIClassInfo::THREADSAFE | nsIClassInfo::SINGLETON,
                   NS_CONSOLESERVICE_CID)
-NS_IMPL_QUERY_INTERFACE_CI(nsConsoleService, nsIConsoleService)
-NS_IMPL_CI_INTERFACE_GETTER(nsConsoleService, nsIConsoleService)
+NS_IMPL_QUERY_INTERFACE_CI(nsConsoleService, nsIConsoleService, nsIObserver)
+NS_IMPL_CI_INTERFACE_GETTER(nsConsoleService, nsIConsoleService, nsIObserver)
 
 static bool sLoggingEnabled = true;
 static bool sLoggingBuffered = true;
 #if defined(ANDROID)
 static bool sLoggingLogcat = true;
 #endif // defined(ANDROID)
 
 
@@ -64,17 +65,17 @@ nsConsoleService::nsConsoleService()
 {
   // XXX grab this from a pref!
   // hm, but worry about circularity, bc we want to be able to report
   // prefs errs...
   mBufferSize = 250;
 }
 
 
-NS_IMETHODIMP
+void
 nsConsoleService::ClearMessagesForWindowID(const uint64_t innerID)
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
 
   // Remove the messages related to this window
   for (uint32_t i = 0; i < mBufferSize && mMessages[i]; i++) {
     // Only messages implementing nsIScriptError interface exposes the inner window ID
     nsCOMPtr<nsIScriptError> scriptError = do_QueryInterface(mMessages[i]);
@@ -87,31 +88,30 @@ nsConsoleService::ClearMessagesForWindow
       continue;
     }
 
     // Free this matching message!
     NS_RELEASE(mMessages[i]);
 
     uint32_t j = i;
     // Now shift all the following messages
+    // XXXkhuey this is not an efficient way to iterate through an array ...
     for (; j < mBufferSize - 1 && mMessages[j + 1]; j++) {
       mMessages[j] = mMessages[j + 1];
     }
     // Nullify the current slot
     mMessages[j] = nullptr;
     mCurrent = j;
 
     // The array is no longer full
     mFull = false;
 
     // Ensure the next iteration handles the messages we just shifted down
     i--;
   }
-
-  return NS_OK;
 }
 
 nsConsoleService::~nsConsoleService()
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
 
   uint32_t i = 0;
   while (i < mBufferSize && mMessages[i]) {
@@ -133,16 +133,22 @@ public:
 
   NS_IMETHOD Run()
   {
     Preferences::AddBoolVarCache(&sLoggingEnabled, "consoleservice.enabled", true);
     Preferences::AddBoolVarCache(&sLoggingBuffered, "consoleservice.buffered", true);
 #if defined(ANDROID)
     Preferences::AddBoolVarCache(&sLoggingLogcat, "consoleservice.logcat", true);
 #endif // defined(ANDROID)
+
+    nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
+    MOZ_ASSERT(obs);
+    obs->AddObserver(mConsole, NS_XPCOM_SHUTDOWN_OBSERVER_ID, false);
+    obs->AddObserver(mConsole, "inner-window-destroyed", false);
+
     if (!sLoggingBuffered) {
       mConsole->Reset();
     }
     return NS_OK;
   }
 
 private:
   nsRefPtr<nsConsoleService> mConsole;
@@ -481,8 +487,31 @@ nsConsoleService::Reset()
    * Free all messages stored so far (cf. destructor)
    */
   for (uint32_t i = 0; i < mBufferSize && mMessages[i]; i++) {
     NS_RELEASE(mMessages[i]);
   }
 
   return NS_OK;
 }
+
+NS_IMETHODIMP
+nsConsoleService::Observe(nsISupports* aSubject, const char* aTopic,
+                          const char16_t* aData)
+{
+  if (!strcmp(aTopic, NS_XPCOM_SHUTDOWN_OBSERVER_ID)) {
+    // Dump all our messages, in case any are cycle collected.
+    Reset();
+    // We could remove ourselves from the observer service, but it is about to
+    // drop all observers anyways, so why bother.
+  } else if (!strcmp(aTopic, "inner-window-destroyed")) {
+    nsCOMPtr<nsISupportsPRUint64> supportsInt = do_QueryInterface(aSubject);
+    MOZ_ASSERT(supportsInt);
+
+    uint64_t windowId;
+    MOZ_ALWAYS_TRUE(NS_SUCCEEDED(supportsInt->GetData(&windowId)));
+
+    ClearMessagesForWindowID(windowId);
+  } else {
+    MOZ_CRASH();
+  }
+  return NS_OK;
+}
diff --git a/xpcom/base/nsConsoleService.h b/xpcom/base/nsConsoleService.h
--- a/xpcom/base/nsConsoleService.h
+++ b/xpcom/base/nsConsoleService.h
@@ -14,24 +14,26 @@
 #include "mozilla/Attributes.h"
 #include "mozilla/Mutex.h"
 
 #include "nsInterfaceHashtable.h"
 #include "nsHashKeys.h"
 
 #include "nsIConsoleService.h"
 
-class nsConsoleService final : public nsIConsoleService
+class nsConsoleService final : public nsIConsoleService,
+                               public nsIObserver
 {
 public:
   nsConsoleService();
   nsresult Init();
 
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSICONSOLESERVICE
+  NS_DECL_NSIOBSERVER
 
   void SetIsDelivering()
   {
     MOZ_ASSERT(NS_IsMainThread());
     MOZ_ASSERT(!mDeliveringMessage);
     mDeliveringMessage = true;
   }
 
@@ -55,16 +57,18 @@ public:
 
   typedef nsInterfaceHashtable<nsISupportsHashKey,
                                nsIConsoleListener> ListenerHash;
   void CollectCurrentListeners(nsCOMArray<nsIConsoleListener>& aListeners);
 
 private:
   ~nsConsoleService();
 
+  void ClearMessagesForWindowID(const uint64_t innerID);
+
   // Circular buffer of saved messages
   nsIConsoleMessage** mMessages;
 
   // How big?
   uint32_t mBufferSize;
 
   // Index of slot in mMessages that'll be filled by *next* log message
   uint32_t mCurrent;
diff --git a/xpcom/base/nsIConsoleService.idl b/xpcom/base/nsIConsoleService.idl
--- a/xpcom/base/nsIConsoleService.idl
+++ b/xpcom/base/nsIConsoleService.idl
@@ -3,17 +3,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsISupports.idl"
 
 interface nsIConsoleListener;
 interface nsIConsoleMessage;
 
-[scriptable, uuid(2436031e-2167-4307-9f1c-a3f38b55a224)]
+[scriptable, uuid(0eb81d20-c37e-42d4-82a8-ca9ae96bdf52)]
 interface nsIConsoleService : nsISupports
 {
     void logMessage(in nsIConsoleMessage message);
 
     /**
      * Convenience method for logging simple messages.
      */
     void logStringMessage(in wstring message);
@@ -39,21 +39,16 @@ interface nsIConsoleService : nsISupport
      * Each registered listener should also be unregistered.
      */
     void unregisterListener(in nsIConsoleListener listener);
 
     /**
      * Clear the message buffer (e.g. for privacy reasons).
      */
     void reset();
-
-    /**
-     * Clear the message buffer for a given inner window.
-     */
-    void clearMessagesForWindowID(in uint64_t innerWindowID);
 };
 
 
 %{ C++
 #define NS_CONSOLESERVICE_CID \
 { 0x7e3ff85c, 0x1dd2, 0x11b2, { 0x8d, 0x4b, 0xeb, 0x45, 0x2c, 0xb0, 0xff, 0x40 }}
 
 #define NS_CONSOLESERVICE_CONTRACTID "@mozilla.org/consoleservice;1"