Bug 1396652: Fix WebRequestService destructor ordering issue. r=mixedpuppy

MozReview-Commit-ID: HYDNYzb5bnM

toolkit/components/extensions/webrequest/WebRequestService.cpp

@@ -14,12 +14,15 @@
 #include "nsISupports.h"
 #include "nsITabParent.h"
 #include "nsITraceableChannel.h"
+#include "nsTArray.h"
 
 #include "mozilla/dom/TabParent.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
 
+static WebRequestService* sWeakWebRequestService;
+
 WebRequestService::WebRequestService()
   : mDataLock("WebRequest service data lock")
 {
@@ -27,10 +30,18 @@ WebRequestService::WebRequestService()
 
 WebRequestService::~WebRequestService()
 {
+  // Store existing entries in an array, since Detach can't safely remove them
+  // while we're iterating the hashtable.
+  AutoTArray<ChannelEntry*, 32> entries;
   for (auto iter = mChannelEntries.Iter(); !iter.Done(); iter.Next()) {
-    auto& channel = iter.Data();
+    entries.AppendElement(iter.Data());
+  }
+
+  for (auto channel : entries) {
     channel->DetachAll();
   }
+
+  sWeakWebRequestService = nullptr;
 }
 
 NS_IMPL_ISUPPORTS(WebRequestService, mozIWebRequestService)
@@ -39,11 +50,16 @@ NS_IMPL_ISUPPORTS(WebRequestService, mozIWebRequestService)
 WebRequestService::GetSingleton()
 {
   static RefPtr<WebRequestService> instance;
-  if (!instance) {
+  if (!sWeakWebRequestService) {
     instance = new WebRequestService();
     ClearOnShutdown(&instance);
+
+    // A separate weak instance that we keep a reference to as long as the
+    // original service is alive, even after our strong reference is cleared to
+    // allow the service to be destroyed.
+    sWeakWebRequestService = instance;
   }
-  return *instance;
+  return *sWeakWebRequestService;
 }
 
 
@@ -139,7 +155,11 @@ WebRequestService::ChannelParent::Detach()
 void
 WebRequestService::ChannelEntry::DetachAll()
 {
-  while (ChannelParent* parent = mTabParents.getFirst()) {
+  // Store the next link gecore calling Detach(), since the last Detach() call
+  // will destroy this instance and poison our mTabParents member.
+  for (ChannelParent *parent, *next = mTabParents.getFirst();
+       (parent = next);) {
+    next = parent->getNext();
     parent->Detach();
   }
 }