Bug 32442 - REGRESSION (r46972): Inspector console outputs the global value for a variable instead of the local value
Summary: REGRESSION (r46972): Inspector console outputs the global value for a variabl...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: HasReduction, InRadar, Regression
: 32513 (view as bug list)
Depends on: 28078
Blocks:
  Show dependency treegraph
 
Reported: 2009-12-11 12:18 PST by David Kilzer (:ddkilzer)
Modified: 2010-03-28 04:42 PDT (History)
8 users (show)

See Also:


Attachments
Test case (777 bytes, text/html)
2009-12-11 12:18 PST, David Kilzer (:ddkilzer)
no flags Details
Proposed fix v1.0 (3.50 KB, patch)
2010-03-27 15:56 PDT, Dmitry Gorbik
pfeldman: review-
Details | Formatted Diff | Diff
Proposed fix v1.1 (3.50 KB, patch)
2010-03-27 16:12 PDT, Dmitry Gorbik
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2009-12-11 12:18:20 PST
Created attachment 44701 [details]
Test case

* SUMMARY
When breaking inside a function with a local variable that masks a global variable, printing the variable shows the globally-scoped variable instead of the locally-scoped one.

* STEPS TO REPRODUCE
1. Launch Safari or WebKit nightly.
2. Enable JavaScript Debugging in the Web Inspector.
3. Load the test page.
4. Type 'x' in the console and hit Enter.

* EXPECTED RESULTS
The console should print:  "This is the local value"

* ACTUAL RESULTS
This console prints:  "This is the global value"

* REGRESSION
This is a regression from shipping Safari 4.0.4.
Comment 1 David Kilzer (:ddkilzer) 2009-12-11 12:57:14 PST
The bisect-builds script reports:

Works: r47686  Fails: r48034

Breakpoints don't seem to work in the Inspector between r47686-r48004, so I couldn't narrow it down in that range.
Comment 2 David Kilzer (:ddkilzer) 2009-12-11 13:00:38 PST
CC-ing some Web Inspector developers.
Comment 3 Sam Weinig 2009-12-26 17:35:30 PST
*** Bug 32513 has been marked as a duplicate of this bug. ***
Comment 4 David Kilzer (:ddkilzer) 2010-01-26 11:23:35 PST
This appears to be a JavaScriptCore bug, not a Web Inspector bug, since it doesn't reproduce on Google Chrome for Mac 4.0.249.49.

Still occurs with WebKit nightly build r53845.
Comment 5 Oliver Hunt 2010-02-01 12:17:08 PST
No longer reproduces in the current nightly
Comment 6 David Kilzer (:ddkilzer) 2010-02-01 13:51:41 PST
(In reply to comment #5)
> No longer reproduces in the current nightly

I still see "This is the global value" (which is the broken behavior) with WebKit nightly r54122.

Which nightly are you using, Oliver?
Comment 7 David Kilzer (:ddkilzer) 2010-02-01 13:53:02 PST
<rdar://problem/7464737>
Comment 8 Oliver Hunt 2010-02-01 13:54:57 PST
(In reply to comment #6)
> (In reply to comment #5)
> > No longer reproduces in the current nightly
> 
> I still see "This is the global value" (which is the broken behavior) with
> WebKit nightly r54122.
> 
> Which nightly are you using, Oliver?

Wrong bug sorry :-(
Comment 9 Martin Häcker 2010-02-25 02:31:39 PST
Any progress here? I just spent another day hunting a bug in the wrong direction because of this. :/

Is there any help that you need? (Another testcase reduction perhpas?)
Comment 10 Dmitry Gorbik 2010-03-27 15:56:04 PDT
Created attachment 51845 [details]
Proposed fix v1.0
Comment 11 Pavel Feldman 2010-03-27 16:07:51 PDT
Comment on attachment 51845 [details]
Proposed fix v1.0

Looks great! One nit and we can land this!

> +    if (!dontUseCommandLineAPI) {
> +        expression = "with (window.console._inspectorCommandLineAPI) { with (window) {\n" + expression + "\n} }";
> +    }

Single-line conditions should not have {}.
Comment 12 Dmitry Gorbik 2010-03-27 16:12:21 PDT
Created attachment 51846 [details]
Proposed fix v1.1
Comment 13 WebKit Commit Bot 2010-03-27 19:32:56 PDT
Comment on attachment 51846 [details]
Proposed fix v1.1

Clearing flags on attachment: 51846

Committed r56676: <http://trac.webkit.org/changeset/56676>
Comment 14 WebKit Commit Bot 2010-03-27 19:33:01 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 David Kilzer (:ddkilzer) 2010-03-28 03:51:02 PDT
Comment on attachment 51846 [details]
Proposed fix v1.1

>+        Fix the regression caused by r28078: a global variable
>+        definition masks a local one in an inspector console
>+        https://bugs.webkit.org/show_bug.cgi?id=32442

r28078 doesn't seem to the be revision that caused the regression.
<http://trac.webkit.org/changeset/28078>
Comment 16 David Kilzer (:ddkilzer) 2010-03-28 04:06:23 PDT
(In reply to comment #15)
> (From update of attachment 51846 [details])
> >+        Fix the regression caused by r28078: a global variable
> >+        definition masks a local one in an inspector console
> >+        https://bugs.webkit.org/show_bug.cgi?id=32442
> 
> r28078 doesn't seem to the be revision that caused the regression.
> <http://trac.webkit.org/changeset/28078>

Ah, I think you mean Bug 28078 which was fixed by r46972.
<http://trac.webkit.org/changeset/46972>
Comment 17 David Kilzer (:ddkilzer) 2010-03-28 04:18:05 PDT
Nice job on the fix, Dmitry!

Is it possible to write a LayoutTests/inspector test for this so it doesn't happen again?
Comment 18 Pavel Feldman 2010-03-28 04:42:32 PDT
(In reply to comment #17)
> Nice job on the fix, Dmitry!
> 
> Is it possible to write a LayoutTests/inspector test for this so it doesn't
> happen again?

Technically, it is possible. However, we don't yet have layout tests that stop in debugger, so various problems are likely to arise.

Scenario for the test would be:
- In the layout test page, create a function that has debugger; statement. That's pretty much all you do on the layout page side, rest is happening on the inspector front-end.
- On the front-end side, enable debugger (this is done synchronously) and execute the function you created in previous step using InjectedScriptAccess.evaluate.
- Just before you do this evaluate, add a sniffer that would give your code control upon WebInspector.pausedScript call from the backend.
- As soon as pausedScript is called, get a reference to the call frame.
- Either synchronously or in setTimeout(0) execute InjectedScriptAccess.evaluateInCallFrame against your call frame with "x" expression.

tests are located under LayoutTests/inspector