1303118 - Webconsole isn't evaluating at selected frame when paused

Status: RESOLVED FIXED

(DevTools :: Console, defect, P1)

Description

[Brian Grinstead [:bgrins]]

STR:

Open data:text/html,<script>var foo = { bar: function() { console.log(this); debugger; }}; foo.bar()</script>
Open debugger and reload page to pause
Evaluate 'this' in console

Expected:
The 'foo' object is returned

Actual:
The window object is returned

This appears to be working in Dev Edition 50 and flipping the old debugger back on doesn't seem to resolve it

Comment 1

[Brian Grinstead [:bgrins]]

Based on regression tracking by :jlast this seems to have started in bug 1263355:

11:57 17:02.81 INFO: Last good revision: 181336fdda6625d8ffa5e5764b817cc3da1f9659
11:57 17:02.81 INFO: First bad revision: bd702fa23037799ab4dd266d8a2b59d021f6cfa8
11:57 17:02.81 INFO: Pushlog:
11:57 https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=181336fdda6625d8ffa5e5764b817cc3da1f9659&tochange=bd702fa23037799ab4dd266d8a2b59d021f6cfa8
11:57 17:04.04 INFO: Looks like the following bug has the changes which introduced the regression:
11:57 https://bugzilla.mozilla.org/show_bug.cgi?id=1263355

Shu, can you please check into this and see why debugger evaluation at a frame isn't working in this case?  The relevant call to frame.evalWithBindings is here: https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/webconsole.js#1321-1323.

Comment 2

[Brian Grinstead [:bgrins]]

Requesting tracking since this causes inaccurate console evaluations when the debugger is paused

Comment 3

[Marcia Knous [:marcia]]

Tracking 51+ for this regression - Brian's reasoning in Comment 2 makes sense.

Comment 4

[Shu-yu Guo [:shu]]

Attachment: Fix 'this' computation for Debugger.Frame.evalWithBindings.

Comment 5

[Eric Faust [:efaust]]

Comment on attachment 8791808 [details] [diff] [review]
Fix 'this' computation for Debugger.Frame.evalWithBindings.

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

::: js/src/frontend/Parser.cpp
@@ +288,5 @@
>      // Debugger.Frame is a function frame. Recompute the this binding if we
>      // are such an eval.
> +    if (enclosingEnv && enclosingScope->kind() == ScopeKind::NonSyntactic) {
> +        // For Debugger.Frame.eval with bindings, the environment chain may
> +        // have more than the DebugEnvironmentProxy. Find that proxy.

I don't understand the last sentence here. This loops isn't really looking for the proxy, just handling it as if it were a environment object, right?

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9f2cb674778
Fix 'this' computation for Debugger.Frame.evalWithBindings. (r=efaust)

Comment 7

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/dcebf2b98e14 for https://treeherder.mozilla.org/logviewer.html#?job_id=36171340&repo=mozilla-inbound

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a89a84c1b7d
Fix 'this' computation for Debugger.Frame.evalWithBindings. (r=efaust)

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1a89a84c1b7d

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1a89a84c1b7d

Comment 11

[Gerry Chang [:gchang]]

Hi :shu,
Since this bug is a regression and also affects 51, do you consider to uplift this for 51 if this patch is not too risky?

Comment 12

[Shu-yu Guo [:shu]]

Comment on attachment 8791808 [details] [diff] [review]
Fix 'this' computation for Debugger.Frame.evalWithBindings.

Approval Request Comment
[Feature/regressing bug #]: 1263355 
[User impact if declined]: Incorrect behavior in webconsole sometimes when evaluating 'this'
[Describe test coverage new/current, TreeHerder]: On m-c, TH
[Risks and why]: Low
[String/UUID change made/needed]: None

Comment 13

[Gerry Chang [:gchang]]

Comment on attachment 8791808 [details] [diff] [review]
Fix 'this' computation for Debugger.Frame.evalWithBindings.

Fix a regression related to webconsole. Take it in 51 aurora.

Comment 14

[Md Maruf Rahman]

I have reproduced this bug with Nightly 51.0a1 (2016-09-15) (64-bit) on Windows 7 , 64 Bit !

This bug's fix is verified with latest Nightly

Build ID   : 20160928030201
User Agent : Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:52.0) Gecko/20100101 Firefox/52.0
[bugday-20160928]

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/90e68a83b08a