Bug 150753 - Web Inspector: Test Debugger.scriptParsed events received after opening inspector frontend
Summary: Web Inspector: Test Debugger.scriptParsed events received after opening inspe...
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: 2015-10-30 20:08 PDT by Joseph Pecoraro
Modified: 2015-11-02 11:35 PST (History)
12 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (9.52 KB, patch)
2015-10-30 20:13 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2015-10-30 20:08:09 PDT
* SUMMARY
Test Debugger.sourceParsed events received after opening inspector frontend.

Previously we were seeing built-in scripts which caused a flurry of unexpected scriptParsed messages being sent to the frontend.

Write a script to test:

    (1) what scriptParsed messages get sent to the frontend as we expect this to be small
    (2) ensure different kinds of scripts (external, inline, attributes, eval, etc) get sent to the frontend as we would expect

This caught an issue with the recent sourceURL/sourceMappingURL directive handling.
Comment 1 Radar WebKit Bug Importer 2015-10-30 20:08:58 PDT
<rdar://problem/23343418>
Comment 2 Joseph Pecoraro 2015-10-30 20:13:46 PDT
Created attachment 264463 [details]
[PATCH] Proposed Fix
Comment 3 WebKit Commit Bot 2015-10-30 21:59:18 PDT
Comment on attachment 264463 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 264463

Committed r191839: <http://trac.webkit.org/changeset/191839>
Comment 4 WebKit Commit Bot 2015-10-30 21:59:23 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 BJ Burg 2015-10-31 08:55:47 PDT
Comment on attachment 264463 [details]
[PATCH] Proposed Fix

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

This is a nice test. Does it cover the JSC change as well?

> LayoutTests/inspector/debugger/scriptParsed.html:86
> +                        foundInjectedScriptSourceScript = true;

I suppose this could be foundXXX = foundXXX || isXXX(), but it doesn't read any better. I would have done foundXXX |= isXXX() since it isn't that expensive to recheck. Alternatively, you could run Array.some() with a lambda for each check.
Comment 6 Joseph Pecoraro 2015-11-02 11:35:11 PST
(In reply to comment #5)
> Comment on attachment 264463 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=264463&action=review
> 
> This is a nice test. Does it cover the JSC change as well?

Yes. It was required to get the sourceURL for the InjectedScript and CommandLineAPI injected script. See the ChangeLog for more details.


> > LayoutTests/inspector/debugger/scriptParsed.html:86
> > +                        foundInjectedScriptSourceScript = true;
> 
> I suppose this could be foundXXX = foundXXX || isXXX(), but it doesn't read
> any better. I would have done foundXXX |= isXXX() since it isn't that
> expensive to recheck. Alternatively, you could run Array.some() with a
> lambda for each check.

I want this test to ensure we don't see duplicates of the same script. The `foundXXX |= isXXX()` approach wouldn't account for that. I'm sure there are stylistically a bunch of possible solutions, this just seemed the most straight forward.