Closed Bug 1072198 Opened 9 years ago Closed 9 years ago
Browser Remoteness() assigns a new .permanent Key to browsers
Turns out that listening for "oop-browser-crashed" in SessionStore and then adding |browser.permanentKey| to a WeakSet didn't work because |updateBrowserRemoteness()| assigns a new permanentKey. I assume that tearing down the <browser> and re-inserting into the tree somehow causes the field to be re-initialized.
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
Attachment #8494411 - Flags: review?(wmccloskey)
Comment on attachment 8494411 [details] [diff] [review] 0001-Bug-1072198-updateBrowserRemoteness-needs-to-keep-.p.patch Review of attachment 8494411 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/tabbrowser.xml @@ +1461,5 @@ > parent.removeChild(aBrowser); > aBrowser.setAttribute("remote", aShouldBeRemote ? "true" : "false"); > + // Tearing down the browser gives a new permanentKey but we want to > + // keep the old one. Re-set it explicitly after unbinding from DOM. > + aBrowser.permanentKey = permanentKey; I'm actually surprised this works. I would have thought that we'd have to change the permanentKey after adding the element back into the DOM. I guess this is fine if it works, though.
Attachment #8494411 - Flags: review?(wmccloskey) → review+
Yeah, me too. But it does. That reminds me I should add a test to ensure that :) I did that explicitly before putting it back into the DOM to have a correct .permanentKey before the first setupSyncHandler message arrives, which I believe would be synchronous at least with an InProcessTabChildGlobal.
https://hg.mozilla.org/integration/fx-team/rev/e16c4c4eb07a Extended browser_e10s_switchbrowser.js to ensure this works and doesn't break in the future. Ensured locally that the test does indeed break without the patch applied.
Iteration: --- → 35.2
Points: --- → 2
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
You need to log in before you can comment on or make changes to this bug.