Bug 161656 - Web Inspector: step-into `console.log(o)` should not step through inspector javascript
Summary: Web Inspector: step-into `console.log(o)` should not step through inspector j...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-09-06 17:00 PDT by Joseph Pecoraro
Modified: 2016-10-12 21:48 PDT (History)
13 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (12.59 KB, patch)
2016-10-12 15:38 PDT, Joseph Pecoraro
timothy: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-yosemite (1.03 MB, application/zip)
2016-10-12 16:24 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-yosemite-wk2 (1.32 MB, application/zip)
2016-10-12 16:56 PDT, Build Bot
no flags Details
[PATCH] For Landing (13.92 KB, patch)
2016-10-12 16:57 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
[PATCH] For Landing (13.08 KB, patch)
2016-10-12 16:59 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews116 for mac-yosemite (2.29 MB, application/zip)
2016-10-12 17:06 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2016-09-06 17:00:33 PDT
Summary:
step-into `console.log(o)` should not step through inspector javascript

Test:
1. <script>
2. console.log(window);
3. <script>

Steps to Reproduce:
1. Inspect test case
2. Set a breakpoint on line 2
3. Reload
  => Pauses on line 2
4. Step-in
  => Expected to step over the console.log statement, instead stayed on the line (20+ times)

Notes:
- Engineering builds can runtime toggle a debug UI to see and step through InjectedScriptSource
  - Maybe this requires a separate backend command to enable/disable this?
Comment 1 Radar WebKit Bug Importer 2016-09-06 17:01:00 PDT
<rdar://problem/28181123>
Comment 2 BJ Burg 2016-09-06 17:23:16 PDT
This seems like a special case of script black boxing.
Comment 3 Joseph Pecoraro 2016-10-12 15:38:29 PDT
Created attachment 291409 [details]
[PATCH] Proposed Fix
Comment 4 Build Bot 2016-10-12 16:24:01 PDT
Comment on attachment 291409 [details]
[PATCH] Proposed Fix

Attachment 291409 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2273212

New failing tests:
inspector/debugger/breakpoint-syntax-error-top-level.html
inspector/debugger/setPauseOnExceptions-uncaught.html
inspector/debugger/command-line-api-exception.html
inspector/dom-debugger/node-removed.html
inspector/debugger/regress-133182.html
inspector/debugger/setPauseOnExceptions-all.html
Comment 5 Build Bot 2016-10-12 16:24:04 PDT
Created attachment 291414 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 6 Joseph Pecoraro 2016-10-12 16:29:48 PDT
Investigating test failures.
Comment 7 Joseph Pecoraro 2016-10-12 16:54:08 PDT
(In reply to comment #6)
> Investigating test failures.

The issue is that SourceID could be 0, and the default HashSet doesn't allow 0. I'm making the following change to allow Map/Set with a SourceID of 0:

-    HashMap<SourceID, DebuggerParseData> m_parseDataMap;
-    HashSet<SourceID> m_blacklistedScripts;
+    HashMap<SourceID, DebuggerParseData, WTF::IntHash<SourceID>, WTF::UnsignedWithZeroKeyHashTraits<SourceID>> m_parseDataMap;
+    HashSet<SourceID, WTF::IntHash<SourceID>, WTF::UnsignedWithZeroKeyHashTraits<SourceID>> m_blacklistedScripts;
Comment 8 Build Bot 2016-10-12 16:56:14 PDT
Comment on attachment 291409 [details]
[PATCH] Proposed Fix

Attachment 291409 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2273307

New failing tests:
inspector/debugger/breakpoint-syntax-error-top-level.html
inspector/debugger/setPauseOnExceptions-uncaught.html
inspector/debugger/command-line-api-exception.html
inspector/dom-debugger/node-removed.html
inspector/debugger/regress-133182.html
inspector/debugger/setPauseOnExceptions-all.html
Comment 9 Build Bot 2016-10-12 16:56:18 PDT
Created attachment 291421 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 10 Joseph Pecoraro 2016-10-12 16:57:29 PDT
Created attachment 291423 [details]
[PATCH] For Landing
Comment 11 Joseph Pecoraro 2016-10-12 16:58:30 PDT
Comment on attachment 291423 [details]
[PATCH] For Landing

View in context: https://bugs.webkit.org/attachment.cgi?id=291423&action=review

> Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.h:161
> +    HashSet<JSC::SourceID> m_blacklistedScripts;

Err, this should be removed.
Comment 12 Joseph Pecoraro 2016-10-12 16:59:08 PDT
Created attachment 291426 [details]
[PATCH] For Landing
Comment 13 Build Bot 2016-10-12 17:06:36 PDT
Comment on attachment 291409 [details]
[PATCH] Proposed Fix

Attachment 291409 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2273308

New failing tests:
inspector/debugger/breakpoint-syntax-error-top-level.html
inspector/debugger/pause-reason.html
inspector/debugger/setPauseOnExceptions-uncaught.html
inspector/debugger/command-line-api-exception.html
inspector/dom-debugger/node-removed.html
inspector/debugger/pause-on-assert.html
inspector/debugger/csp-exceptions.html
inspector/debugger/regress-133182.html
inspector/debugger/setPauseOnExceptions-all.html
Comment 14 Build Bot 2016-10-12 17:06:40 PDT
Created attachment 291428 [details]
Archive of layout-test-results from ews116 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 15 WebKit Commit Bot 2016-10-12 19:08:21 PDT
Comment on attachment 291426 [details]
[PATCH] For Landing

Clearing flags on attachment: 291426

Committed r207267: <http://trac.webkit.org/changeset/207267>