From 1230305e9a8f5b723ad1c12b0a818326dd83634c Mon Sep 17 00:00:00 2001
From: Hiroyuki Ikezoe <hiikezoe@mozilla-japan.org>
Date: Thu, 20 Aug 2015 14:33:54 -0700
Subject: [PATCH] Bug 1195700 - Disconnect performance observer before being
 destroyed to avoid crash. r=baku

---
 dom/base/PerformanceObserver.cpp           | 11 ++++++++++-
 dom/base/PerformanceObserver.h             |  1 +
 dom/base/test/performance_observer.html    |  1 +
 dom/base/test/test_performance_observer.js |  7 +++++++
 4 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/dom/base/PerformanceObserver.cpp b/dom/base/PerformanceObserver.cpp
index 21d7741b1bbc..f3f9017f74c8 100644
--- a/dom/base/PerformanceObserver.cpp
+++ b/dom/base/PerformanceObserver.cpp
@@ -38,6 +38,7 @@ PerformanceObserver::PerformanceObserver(nsPIDOMWindow* aOwner,
                                          PerformanceObserverCallback& aCb)
   : mOwner(aOwner)
   , mCallback(&aCb)
+  , mConnected(false)
 {
   MOZ_ASSERT(mOwner);
   mPerformance = aOwner->GetPerformance();
@@ -46,6 +47,7 @@ PerformanceObserver::PerformanceObserver(nsPIDOMWindow* aOwner,
 PerformanceObserver::PerformanceObserver(WorkerPrivate* aWorkerPrivate,
                                          PerformanceObserverCallback& aCb)
   : mCallback(&aCb)
+  , mConnected(false)
 {
   MOZ_ASSERT(aWorkerPrivate);
   mPerformance = aWorkerPrivate->GlobalScope()->GetPerformance();
@@ -53,6 +55,8 @@ PerformanceObserver::PerformanceObserver(WorkerPrivate* aWorkerPrivate,
 
 PerformanceObserver::~PerformanceObserver()
 {
+  Disconnect();
+  MOZ_ASSERT(!mConnected);
 }
 
 // static
@@ -145,10 +149,15 @@ PerformanceObserver::Observe(const PerformanceObserverInit& aOptions,
   mEntryTypes = validEntryTypes;
 
   mPerformance->AddObserver(this);
+  mConnected = true;
 }
 
 void
 PerformanceObserver::Disconnect()
 {
-  mPerformance->RemoveObserver(this);
+  if (mConnected) {
+    MOZ_ASSERT(mPerformance);
+    mPerformance->RemoveObserver(this);
+    mConnected = false;
+  }
 }
diff --git a/dom/base/PerformanceObserver.h b/dom/base/PerformanceObserver.h
index 0a7fcd60330f..85de6ea02d53 100644
--- a/dom/base/PerformanceObserver.h
+++ b/dom/base/PerformanceObserver.h
@@ -68,6 +68,7 @@ private:
   nsRefPtr<PerformanceObserverCallback> mCallback;
   nsRefPtr<PerformanceBase> mPerformance;
   nsTArray<nsString> mEntryTypes;
+  bool mConnected;
 };
 
 } // namespace dom
diff --git a/dom/base/test/performance_observer.html b/dom/base/test/performance_observer.html
index d194c17e42df..b24479d8c924 100644
--- a/dom/base/test/performance_observer.html
+++ b/dom/base/test/performance_observer.html
@@ -48,6 +48,7 @@ async_test(t => {
     observedEntryList = list;
   });
   observer.observe({entryTypes: ['resource']});
+  t.add_cleanup(() => observer.disconnect());
 
   assert_equals(observedEntries.length, 0);
 
diff --git a/dom/base/test/test_performance_observer.js b/dom/base/test/test_performance_observer.js
index 5eb43c10ceec..cef0f33cd178 100644
--- a/dom/base/test/test_performance_observer.js
+++ b/dom/base/test/test_performance_observer.js
@@ -44,6 +44,7 @@ test(t => {
     list.getEntries().forEach(entry => observedEntries.push(entry));
   });
   observer.observe({entryTypes: ['mark', 'measure']});
+  t.add_cleanup(() => observer.disconnect());
 
   assert_equals(observedEntries.length, 0,
                 "User timing entries should never be observed.");
@@ -107,6 +108,7 @@ test(t => {
     observedEntryList = list;
   });
   observer.observe({entryTypes: ['mark']});
+  t.add_cleanup(() => observer.disconnect());
 
   performance.mark("test");
   assert_array_equals(observedEntryList.getEntries({"entryType": "mark"}),
@@ -150,6 +152,7 @@ test(t => {
     observedEntryList = list;
   });
   observer.observe({entryTypes: ['mark', 'measure']});
+  t.add_cleanup(() => observer.disconnect());
 
   performance.mark("test");
   assert_array_equals(observedEntryList.getEntriesByType("mark"),
@@ -169,6 +172,7 @@ test(t => {
     observedEntryList = list;
   });
   observer.observe({entryTypes: ['mark', 'measure']});
+  t.add_cleanup(() => observer.disconnect());
 
   performance.mark("test");
   assert_array_equals(observedEntryList.getEntriesByName("test"),
@@ -190,6 +194,7 @@ test(t => {
 
   observer.observe({entryTypes: ['mark', 'measure']});
   observer.observe({entryTypes: ['mark', 'measure']});
+  t.add_cleanup(() => observer.disconnect());
 
   performance.mark("test-start");
   performance.mark("test-end");
@@ -210,6 +215,7 @@ test(t => {
 
   observer.observe({entryTypes: ['mark', 'measure']});
   observer.observe({entryTypes: ['mark']});
+  t.add_cleanup(() => observer.disconnect());
 
   performance.mark("test-start");
   performance.mark("test-end");
@@ -230,6 +236,7 @@ test(t => {
 
   observer.observe({entryTypes: ['mark']});
   observer.observe({entryTypes: ['measure']});
+  t.add_cleanup(() => observer.disconnect());
 
   performance.mark("test-start");
   performance.mark("test-end");