Victor Porof [:vporof][:vp] Assignee
	Description • 12 years ago

Although it's disabled by default, it's something we should fix.
Afaict, the remoteIncomingPromptMessage dialog never shows up, resulting in the DebuggerServer never initializing, resulting in the second process throwing up some errors and never being able to close.

PS, after making it work again, we also need to change
Services.prefs.setBoolPref("devtools.debugger.remote-enabled", false);
to also be accompanied by a 
Services.prefs.setBoolPref("devtools.debugger.chrome-enabled", false);

	Victor Porof [:vporof][:vp] Assignee
	Comment 1 • 12 years ago

Actually a regression from bug 751949.

	Victor Porof [:vporof][:vp] Assignee
	Comment 3 • 12 years ago

Interestingly enough, the simple change
-    return !("content" in window.parent) && !this._isRemoteDebugger;
+    let hasContent;
+    try {
+      hasContent = !!window.parent.content;
+    } catch (e) {
+      // If an exception was thrown, we can't rely on the result to be accurate.
+      return false;
+    }
+    return !hasContent && !this._isRemoteDebugger;

causes leaks...
This is pretty obvious now, since before this patch !("content" in window.parent) would always be false, hence the function would always return false. Things are different now and the function returns The Right Thing in some cases, instead of The Wrong Thing in all cases.

Investigating...

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_menustatus.js | leaked until shutdown [nsGlobalWindow #242 about:blank]
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_panesize-inner.js | leaked until shutdown [nsGlobalWindow #280 about:blank]

Both tests share the same pattern of listening for Debugger:Loaded, doing some stuff, and then handling Debugger.Unloaded for cleanup. We're in callback hell again.

	Victor Porof [:vporof][:vp] Assignee
	Comment 4 • 12 years ago

Even more, these leaks didn't appear before because of TypeErrors.

JavaScript error: chrome://browser/content/debugger-controller.js, line 265: TypeError: invalid 'in' operand window.parent

	Victor Porof [:vporof][:vp] Assignee
	Comment 5 • 12 years ago

Removed previous remote/chrome checks with something much much simpler.
Fixed tests.

JAVASCRIPT!

	Panos Astithas (he/him) [:past] (please ni?)
	Comment 6 • 12 years ago

Comment on attachment 660775 [details] [diff] [review]
v2

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

::: browser/devtools/debugger/DebuggerUI.jsm
@@ +494,5 @@
>      }
>      if (result == 2) {
>        DebuggerServer.closeListener();
>        Services.prefs.setBoolPref("devtools.debugger.remote-enabled", false);
> +      Services.prefs.setBoolPref("devtools.debugger.chrome-enabled", false);

As discussed in IRC, I don't think we want to do this.

	Victor Porof [:vporof][:vp] Assignee
	Comment 7 • 12 years ago

https://hg.mozilla.org/integration/fx-team/rev/69848e5d3672

	Victor Porof [:vporof][:vp] Assignee
	Comment 8 • 12 years ago

Whoops, switched the wrong pref when landed, fixed:
https://hg.mozilla.org/integration/fx-team/rev/3f7d16ee5712

	Tim Taubert [:ttaubert] (inactive)
	Comment 9 • 12 years ago

https://hg.mozilla.org/mozilla-central/rev/69848e5d3672
https://hg.mozilla.org/mozilla-central/rev/3f7d16ee5712