Bug 136806 - Web Inspector: InjectedScripts should not be profiled or displayed in Timeline
Summary: Web Inspector: InjectedScripts should not be profiled or displayed in Timeline
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brian Burg
URL:
Keywords: InRadar
Depends on: 136805
Blocks: WebReplayUI
  Show dependency treegraph
 
Reported: 2014-09-14 11:46 PDT by Brian Burg
Modified: 2014-09-29 20:34 PDT (History)
5 users (show)

See Also:


Attachments
WIP (8.83 KB, patch)
2014-09-24 16:15 PDT, Brian Burg
no flags Details | Formatted Diff | Diff
Patch (8.68 KB, patch)
2014-09-29 16:22 PDT, Brian Burg
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Burg 2014-09-14 11:46:49 PDT
It doesn't make sense to show these in the timeline, at least for inspector^1. They only really show up when debugging, they generate a lot of profile data, and are sort of irrelevant for the purpose of profiling user content.

I can think of some uses for seeing injected scripts in the timeline for inspector^2 and above. But, I'm not quite sure how to distinguish that from inside the engine (look at the PageGroup or something?)
Comment 1 Radar WebKit Bug Importer 2014-09-14 11:46:57 PDT
<rdar://problem/18332123>
Comment 2 Timothy Hatcher 2014-09-15 11:09:33 PDT
Good point. We do hide the InjectedScript from backtraces in the Debugger. We should be able to do it for the profiler too.

It would make sense to show it for Inspector^2. That would be via the PageGroup identifier.
Comment 3 Brian Burg 2014-09-15 12:15:30 PDT
(In reply to comment #2)
> Good point. We do hide the InjectedScript from backtraces in the Debugger. We should be able to do it for the profiler too.
> 
> It would make sense to show it for Inspector^2. That would be via the PageGroup identifier.

The profiler can distinguish different page group ids, but I don't think the inspector "level" is exposed anywhere outside of UIProcess. What's a good way to expose that? WebInspectorClient?

My WIP patch has a ProfileGenerator::setSuspended flag that is set and unset under the InspectorController will/didCallInjectedScript.
Comment 4 Brian Burg 2014-09-24 16:15:43 PDT
Created attachment 238621 [details]
WIP
Comment 5 Timothy Hatcher 2014-09-24 21:31:00 PDT
Comment on attachment 238621 [details]
WIP

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

Makes sense to me.

> Source/WebCore/inspector/InspectorController.cpp:-427
> -    InspectorInstrumentationCookie cookie = InspectorInstrumentation::willCallFunction(scriptExecutionContext, scriptName, scriptLine);

Yeah I never understood why this was instrumented.
Comment 6 Brian Burg 2014-09-29 16:22:33 PDT
Created attachment 238895 [details]
Patch
Comment 7 WebKit Commit Bot 2014-09-29 16:23:33 PDT
Attachment 238895 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/profiler/ProfileGenerator.cpp:53:  Wrong number of spaces before statement. (expected: 8)  [whitespace/indent] [4]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 WebKit Commit Bot 2014-09-29 20:34:16 PDT
Comment on attachment 238895 [details]
Patch

Clearing flags on attachment: 238895

Committed r174095: <http://trac.webkit.org/changeset/174095>
Comment 9 WebKit Commit Bot 2014-09-29 20:34:19 PDT
All reviewed patches have been landed.  Closing bug.