The profile which used to live in the timeline tab is now gone completely. Starting profiles programmatically is also impossible; console.profile() returns undefined
<rdar://problem/17566028>
Profiles are now part of the Timeline and are captured when the timeline is recording. To see the profile data, click on the JavaScript and Events timeline which will show a data grid of all the recorded profiles. Indeed, console.profile() is not working and I am working on making it record into the the timeline.
Created attachment 235068 [details] Patch
Comment on attachment 235068 [details] Patch Attachment 235068 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/4891632147103744 New failing tests: inspector-protocol/profiler/console-profileEnd-parameterless.html inspector-protocol/profiler/console-profile.html
Created attachment 235072 [details] Archive of layout-test-results from webkit-ews-08 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-08 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 235068 [details] Patch Attachment 235068 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/4632379331182592 New failing tests: inspector-protocol/profiler/console-profileEnd-parameterless.html media/W3C/video/networkState/networkState_during_loadstart.html media/track/track-long-word-container-sizing.html inspector-protocol/profiler/console-profile.html
Created attachment 235074 [details] Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-15 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 235068 [details] Patch Attachment 235068 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/4599771939471360 New failing tests: inspector-protocol/profiler/console-profileEnd-parameterless.html inspector-protocol/profiler/console-profile.html
Created attachment 235077 [details] Archive of layout-test-results from webkit-ews-05 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-05 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 235068 [details] Patch Attachment 235068 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5103794136285184 New failing tests: inspector-protocol/profiler/console-profileEnd-parameterless.html media/W3C/video/networkState/networkState_during_loadstart.html
Created attachment 235080 [details] Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-09 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 235068 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=235068&action=review r=me > Source/WebCore/inspector/InspectorController.cpp:386 > void InspectorController::setProfilerEnabled(bool enable) This is now only for layout tests. Maybe we should rename it, or at least include a comment so it is less ambiguous. Was this called by any other ports, or just mac? > Source/WebCore/inspector/InspectorTimelineAgent.cpp:161 > + // the tile of an already recording profile. Typo: "tile" => "title" > Source/WebCore/inspector/InspectorTimelineAgent.cpp:165 > + record.data->getString("title", &recordTitle); Nit: ASCIILiteral("title") > Source/WebCore/inspector/InspectorTimelineAgent.cpp:183 > + for (ptrdiff_t i = m_pendingConsoleProfileRecords.size() - 1; i >= 0; --i) { Nit: ptrdiff_t? Sounds like you want just "int", or am I missing something? > Source/WebCore/inspector/InspectorTimelineAgent.cpp:187 > + record.data->getString("title", &recordTitle); Nit: ASCIILiteral("title") > Source/WebCore/inspector/InspectorTimelineAgent.cpp:645 > + entry.record->setObject("data", entry.data); > + entry.record->setArray("children", entry.children); > + entry.record->setNumber("endTime", timestamp()); Nit: ASCIILiteral > Source/WebCore/inspector/InspectorTimelineAgent.h:235 > + TimelineRecordEntry() { } Nit: Initialize type to something, so it isn't random. > Source/WebCore/inspector/InspectorTimelineAgent.h:286 > + unsigned m_enabledFromConsole; Nit: Unused? Seems like this can be removed.
Created attachment 235087 [details] Patch
Created attachment 235089 [details] Patch
Comment on attachment 235089 [details] Patch Clearing flags on attachment: 235089 Committed r171195: <http://trac.webkit.org/changeset/171195>
All reviewed patches have been landed. Closing bug.
This is causing fast/profiler/profile-with-no-title to crash in Debug builds, the JSC LegacyProfiler is asserting !title.isNull(). <http://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK1%20(Tests)/r171195%20(6522)/fast/profiler/profile-with-no-title-crash-log.txt> It looks like the Legacy Profiler allows null titles in stop profiling, and the tests pass in release, so it seems fine to remove the assert. But Tim, you may want to look deeper into this.
Landed <http://trac.webkit.org/changeset/171204>
Thanks for the fix. It is correct that empty / null should work. In the past ConsoleAgent would substitute empty / null with a generated string. We don't do that now and just handle empty strings in the UI.
Comment on attachment 235089 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=235089&action=review > Source/WebCore/bindings/js/ScriptState.cpp:62 > + ScriptExecutionContext* context = scriptExecutionContextFromExecState(scriptState); > + Document* document = context && context->isDocument() ? toDocument(context) : nullptr; > + return document ? document->frame() : nullptr; This includes an extra null check that isn’t helpful. I think it should just be two lines: ScriptExecutionContext* context = scriptExecutionContextFromExecState(scriptState); return context && context->isDocument() ? toDocument(context)->frame() : nullptr; > Source/WebCore/inspector/InspectorTimelineAgent.cpp:184 > + for (ptrdiff_t i = m_pendingConsoleProfileRecords.size() - 1; i >= 0; --i) { > + const TimelineRecordEntry& record = m_pendingConsoleProfileRecords[i]; We really need a better idiom for iterating in reverse order. This ptrdiff_t and -1 stuff is super ugly. I want this to be: for (auto& record : m_pendingConsoleProfileRecords.reversed()) Or something like that. > Source/WebCore/inspector/InspectorTimelineAgent.cpp:334 > + const TimelineRecordEntry& entry = m_recordStack.last(); > entry.data->setNumber("endLine", endLine); Why do we need a local at all? Putting this into a single line might be cleaner. > Source/WebCore/inspector/InspectorTimelineAgent.cpp:627 > + const TimelineRecordEntry& parent = m_recordStack.last(); > parent.children->pushObject(record.release()); Why do we need a local at all? Putting this into a single line might be cleaner. > Source/WebCore/inspector/InspectorTimelineAgent.cpp:653 > if (!m_recordStack.isEmpty()) { Should be early return. > Source/WebCore/inspector/InspectorTimelineAgent.h:145 > + virtual void start(ErrorString* = nullptr, const int* maxCallStackDepth = nullptr) override; > + virtual void stop(ErrorString* = nullptr) override; Seems strange to needs these default values in an override. Are people actually calling these functions on this class rather than on the base class? In a related question, can this class be marked final? > Source/WebCore/inspector/InspectorTimelineAgent.h:236 > + TimelineRecordEntry() > + : type(TimelineRecordType::EventDispatch) { } I’d put the braces vertically to match the other constructor just below. If the check-webkit-style script allowed the “all in one line” style, that would be OK with me too. But to me this looks like a hybrid. > Source/WebCore/inspector/InspectorTimelineAgent.h:253 > + TimelineRecordEntry createRecordEntry(PassRefPtr<Inspector::InspectorObject> data, TimelineRecordType, bool captureCallStack, Frame*); Not sure the argument name “data” here is helpful. > Source/WebInspectorUI/UserInterface/Images/TimelineRecordConsoleProfile.svg:2 > +<!-- Copyright © 2013 Apple Inc. All rights reserved. --> Copyright 2013? > Source/WebKit/mac/WebInspector/WebInspector.mm:113 > + // No longer supported. > + return NO; Are we going to be able to delete these ever? If so, when? > Source/WebKit2/WebProcess/WebPage/WebInspector.cpp:280 > + // No longer supported. Same question as above.
*** Bug 117571 has been marked as a duplicate of this bug. ***