860176 - Frameworker's document-element-inserted not removed if worker is closed quickly

Status: RESOLVED FIXED

(Firefox Graveyard :: SocialAPI, defect)

Description

[Mark Hammond [:markh] [:mhammond]]

Stumbled across this.  Add the following patch to Frameworker:

     Services.obs.addObserver(function injectController(doc, topic, data) {
+      dump("injectController fired for " + self.name + " - " + doc.location.href + "\n");
       if (!doc.defaultView || doc.defaultView != self.frame.contentWindow) {
         return;
       }
+      dump("injectController is ours\n");
       Services.obs.removeObserver(injectController, "document-element-inserted");
       try {
         self.createSandbox();
       } catch (e) {
         Cu.reportError("FrameWorker: failed to create sandbox for " + url + ". " + e);
       }
     }, "document-element-inserted", false);
+    dump("added injectController for " + this.name + " - " + this.url + "\n");

and run the frameworker tests.  By the end of the tests you see:

added injectController for SocialProvider:http://example.com - http://example.com/browser/toolkit/components/social/test/browser/worker_social.js
injectController fired for SocialProvider:http://example.com - http://example.com/browser/toolkit/components/social/test/browser/worker_social.js
injectController is ours
injectController fired for testEarlyClose - http://example.com/browser/toolkit/components/social/test/browser/worker_social.js
injectController fired for SocialProvider:http://example.com - http://example.com/browser/toolkit/components/social/test/browser/worker_social.js
injectController fired for SocialProvider:http://example.com - http://example.com/browser/toolkit/components/social/test/browser/worker_social.js

Meaning there are a number of observers which were added but are never removed.  I'm guessing this is a result of us closing the worker in some cases before it has fully initialized.

Comment 1

[Mark Hammond [:markh] [:mhammond]]

Attachment: Check if we need to remove observer in terminate() method

This patch tracks if the observer has fired and removed itself, and if not, removes it in the terminate() method.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 737352 [details] [diff] [review]
Check if we need to remove observer in terminate() method

Review of attachment 737352 [details] [diff] [review]:
-----------------------------------------------------------------

Nice fix!

Comment 3

[Mark Hammond [:markh] [:mhammond]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8705058b43fd

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8705058b43fd