[webkit-reviews] review granted: [Bug 134643] Web Inspector: Profiler disappeared : [Attachment 235068] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Jul 17 13:17:04 PDT 2014
Joseph Pecoraro <joepeck at webkit.org> has granted Timothy Hatcher
<timothy at apple.com>'s request for review:
Bug 134643: Web Inspector: Profiler disappeared
https://bugs.webkit.org/show_bug.cgi?id=134643
Attachment 235068: Patch
https://bugs.webkit.org/attachment.cgi?id=235068&action=review
------- Additional Comments from Joseph Pecoraro <joepeck at webkit.org>
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.
More information about the webkit-reviews
mailing list