Bug 52869 – Web Inspector: switch page/Console implementation from InspectorController to InspectorInstrumentation

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Ilya Tikhonovsky]

%subj%

[Ilya Tikhonovsky]

Created attachment 79700 [details]
[patch] initial version

[WebKit Review Bot]

Attachment 79700 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7604260

[Early Warning System Bot]

Attachment 79700 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7522228

[Ilya Tikhonovsky]

Created attachment 79701 [details]
[patch] second iteration

[Build Bot]

Attachment 79700 [details] did not build on win:
Build output: http://queues.webkit.org/results/7602242

[WebKit Review Bot]

Attachment 79700 [details] did not build on mac:
Build output: http://queues.webkit.org/results/7539257

[Yury Semikhatsky]

Comment on attachment 79701 [details]
[patch] second iteration

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

> Source/WebCore/inspector/InspectorInstrumentation.cpp:571
> +    return inspectorController->profilerAgent()->getCurrentUserInitiatedProfileName(incrementProfileNumber);

ProfilerAgent may be 0 here, r- for this.

> Source/WebCore/inspector/InspectorInstrumentation.h:142
> +    static void addProfile(Page*, RefPtr<ScriptProfile>, ScriptCallStack*);

ScriptCallStack should be passed as PassOwnPtr since the profiler agent may want to store the callstack in a console message.

> Source/WebCore/inspector/InspectorInstrumentation.h:902
> +    if (InspectorController* inspectorController = inspectorControllerForPage(page))

There was no check for InspectorController, I don't think it can be 0 here.

[Ilya Tikhonovsky]

Created attachment 79707 [details]
[patch] third iteration

(In reply to comment #7)
> (From update of attachment 79701 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=79701&action=review
> 
> > Source/WebCore/inspector/InspectorInstrumentation.cpp:571
> > +    return inspectorController->profilerAgent()->getCurrentUserInitiatedProfileName(incrementProfileNumber);
> 
> ProfilerAgent may be 0 here, r- for this.

fixed.


> > Source/WebCore/inspector/InspectorInstrumentation.h:142
> > +    static void addProfile(Page*, RefPtr<ScriptProfile>, ScriptCallStack*);
> 
> ScriptCallStack should be passed as PassOwnPtr since the profiler agent may want to store the callstack in a console message.

There are 4 such functions which receiving ScriptCallStack by pointer.
These functions use script name and line number of top frame at the moment. 
This can be implemented in another patch if you want.

> 
> > Source/WebCore/inspector/InspectorInstrumentation.h:902
> > +    if (InspectorController* inspectorController = inspectorControllerForPage(page))
> 
> There was no check for InspectorController, I don't think it can be 0 here.
It is a common practice in InspectorInstrumentation code.

[Ilya Tikhonovsky]

Comment on attachment 79707 [details]
[patch] third iteration

Clearing flags on attachment: 79707

Committed r76335: <http://trac.webkit.org/changeset/76335>

[Ilya Tikhonovsky]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/76335 might have broken SnowLeopard Intel Release (Tests)
The following tests are not passing:
fast/profiler/anonymous-event-handler.html
fast/profiler/anonymous-function-called-from-different-contexts.html
fast/profiler/anonymous-function-calls-built-in-functions.html
fast/profiler/anonymous-function-calls-eval.html
fast/profiler/anonymous-functions-with-display-names.html
fast/profiler/apply.html
fast/profiler/built-in-function-calls-anonymous.html
fast/profiler/built-in-function-calls-user-defined-function.html
fast/profiler/call-nodelist-as-function.html
fast/profiler/call.html
fast/profiler/calling-the-function-that-started-the-profiler-from-another-scope.html
fast/profiler/compare-multiple-profiles.html
fast/profiler/constructor.html
fast/profiler/dead-time.html
fast/profiler/document-dot-write.html
fast/profiler/event-handler.html
fast/profiler/execution-context-and-eval-on-same-line.html
fast/profiler/inline-event-handler.html
fast/profiler/many-calls-in-the-same-scope.html
fast/profiler/multiple-and-different-scoped-anonymous-function-calls.html
fast/profiler/multiple-and-different-scoped-function-calls.html
fast/profiler/multiple-anonymous-functions-called-from-the-same-function.html
fast/profiler/multiple-frames.html
fast/profiler/named-functions-with-display-names.html
fast/profiler/nested-anonymous-functon.html
fast/profiler/nested-start-and-stop-profiler.html
fast/profiler/one-execution-context.html
fast/profiler/profile-calls-in-included-file.html
fast/profiler/profile-with-no-title.html
fast/profiler/profiling-from-a-nested-location-but-stop-profiling-outside-the-nesting.html
fast/profiler/profiling-from-a-nested-location.html
fast/profiler/simple-event-call.html
fast/profiler/simple-no-level-change.html
fast/profiler/start-and-stop-profiler-multiple-times.html
fast/profiler/start-and-stop-profiling-in-the-same-function.html
fast/profiler/stop-profiling-after-setTimeout.html
fast/profiler/stop-then-function-call.html
fast/profiler/two-execution-contexts.html
fast/profiler/user-defined-function-calls-built-in-functions.html
fast/profiler/window-dot-eval.html

[Ilya Tikhonovsky]

actually landed as 

Committed r76344
	M	Source/WebCore/ChangeLog
	M	Source/WebCore/page/Console.cpp
	M	Source/WebCore/inspector/InspectorController.cpp
	M	Source/WebCore/inspector/InspectorController.h
	M	Source/WebCore/inspector/InspectorInstrumentation.cpp
	M	Source/WebCore/inspector/InspectorInstrumentation.h
r76344 = 043f7d3c197b58cfab39413ffbdec0c6cf84955b (refs/remotes/trunk)