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

--HG--
extra : rebase_source : 689ed129fd8df604385dde4fe9d9477f14f2fced

dom/base/nsGlobalWindow.cpp

@@ -42,7 +42,6 @@
 #include "WindowNamedPropertiesHandler.h"
 #include "nsFrameSelection.h"
 #include "nsNetUtil.h"
-#include "nsIConsoleService.h"
 
 // Helper Classes
 #include "nsJSUtils.h"
@@ -1567,12 +1566,6 @@ 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.

dom/ipc/ContentParent.cpp

@@ -4304,13 +4304,14 @@ ContentParent::GetConsoleService()
         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();
 }
 

xpcom/base/nsConsoleService.cpp

@@ -22,6 +22,7 @@
 #include "nsPrintfCString.h"
 #include "nsProxyRelease.h"
 #include "nsIScriptError.h"
+#include "nsISupportsPrimitives.h"
 
 #include "mozilla/Preferences.h"
 
@@ -45,8 +46,8 @@ 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;
@@ -69,7 +70,7 @@ nsConsoleService::nsConsoleService()
 }
 
 
-NS_IMETHODIMP
+void
 nsConsoleService::ClearMessagesForWindowID(const uint64_t innerID)
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
@@ -92,6 +93,7 @@ nsConsoleService::ClearMessagesForWindowID(const uint64_t innerID)
 
     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];
     }
@@ -105,8 +107,6 @@ nsConsoleService::ClearMessagesForWindowID(const uint64_t innerID)
     // Ensure the next iteration handles the messages we just shifted down
     i--;
   }
-
-  return NS_OK;
 }
 
 nsConsoleService::~nsConsoleService()
@@ -138,6 +138,12 @@ public:
 #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();
     }
@@ -486,3 +492,26 @@ nsConsoleService::Reset()
 
   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;
+}

xpcom/base/nsConsoleService.h

@@ -19,7 +19,8 @@
 
 #include "nsIConsoleService.h"
 
-class nsConsoleService final : public nsIConsoleService
+class nsConsoleService final : public nsIConsoleService,
+                               public nsIObserver
 {
 public:
   nsConsoleService();
@@ -27,6 +28,7 @@ public:
 
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSICONSOLESERVICE
+  NS_DECL_NSIOBSERVER
 
   void SetIsDelivering()
   {
@@ -60,6 +62,8 @@ public:
 private:
   ~nsConsoleService();
 
+  void ClearMessagesForWindowID(const uint64_t innerID);
+
   // Circular buffer of saved messages
   nsIConsoleMessage** mMessages;
 

xpcom/base/nsIConsoleService.idl

@@ -8,7 +8,7 @@
 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);
@@ -44,11 +44,6 @@ interface nsIConsoleService : nsISupports
      * 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);
 };