Bug 52869 - Web Inspector: switch page/Console implementation from InspectorController to InspectorInstrumentation
Summary: Web Inspector: switch page/Console implementation from InspectorController to...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Ilya Tikhonovsky
URL:
Keywords:
Depends on: 52875
Blocks:
  Show dependency treegraph
 
Reported: 2011-01-20 22:53 PST by Ilya Tikhonovsky
Modified: 2011-01-22 00:20 PST (History)
18 users (show)

See Also:


Attachments
[patch] initial version (9.60 KB, patch)
2011-01-20 23:09 PST, Ilya Tikhonovsky
no flags Details | Formatted Diff | Diff
[patch] second iteration (8.98 KB, patch)
2011-01-20 23:39 PST, Ilya Tikhonovsky
yurys: review-
Details | Formatted Diff | Diff
[patch] third iteration (9.20 KB, patch)
2011-01-21 01:06 PST, Ilya Tikhonovsky
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ilya Tikhonovsky 2011-01-20 22:53:10 PST
%subj%
Comment 1 Ilya Tikhonovsky 2011-01-20 23:09:52 PST
Created attachment 79700 [details]
[patch] initial version
Comment 2 WebKit Review Bot 2011-01-20 23:16:54 PST
Attachment 79700 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7604260
Comment 3 Early Warning System Bot 2011-01-20 23:23:22 PST
Attachment 79700 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7522228
Comment 4 Ilya Tikhonovsky 2011-01-20 23:39:50 PST
Created attachment 79701 [details]
[patch] second iteration
Comment 5 Build Bot 2011-01-20 23:40:06 PST
Attachment 79700 [details] did not build on win:
Build output: http://queues.webkit.org/results/7602242
Comment 6 WebKit Review Bot 2011-01-21 00:09:37 PST
Attachment 79700 [details] did not build on mac:
Build output: http://queues.webkit.org/results/7539257
Comment 7 Yury Semikhatsky 2011-01-21 00:20:25 PST
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.
Comment 8 Ilya Tikhonovsky 2011-01-21 01:06:45 PST
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.
Comment 9 Ilya Tikhonovsky 2011-01-21 01:24:59 PST
Comment on attachment 79707 [details]
[patch] third iteration

Clearing flags on attachment: 79707

Committed r76335: <http://trac.webkit.org/changeset/76335>
Comment 10 Ilya Tikhonovsky 2011-01-21 01:25:11 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 WebKit Review Bot 2011-01-21 01:58:13 PST
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
Comment 12 Ilya Tikhonovsky 2011-01-22 00:20:58 PST
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)