[webkit-reviews] review granted: [Bug 108653] Web Inspector: Split Profiler domain in protocol into Profiler and HeapProfiler : [Attachment 187071] Patch

[bugzilla-daemon at webkit.org]

Yury Semikhatsky <yurys at chromium.org> has granted Alexei Filippov
<alph at chromium.org>'s request for review:
Bug 108653: Web Inspector: Split Profiler domain in protocol into Profiler and
HeapProfiler
https://bugs.webkit.org/show_bug.cgi?id=108653

Attachment 187071: Patch
https://bugs.webkit.org/attachment.cgi?id=187071&action=review

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=187071&action=review


> Source/WebCore/inspector/InspectorHeapProfilerAgent.cpp:2
> + * Copyright (C) 2010 Apple Inc. All rights reserved.

2013. Please use Google copyright header for new files(it is slightly different
from this one).

> Source/WebCore/inspector/InspectorHeapProfilerAgent.cpp:131
> +public:

Please move classes in the anonymous namespace to the top of the file.

> Source/WebCore/inspector/InspectorHeapProfilerAgent.h:2
> + * Copyright (C) 2010 Apple Inc. All rights reserved.

Same comment about copyright header as above.

> Source/WebCore/inspector/InspectorHeapProfilerAgent.h:58
> +    void addProfileFinishedMessageToConsole(PassRefPtr<ScriptProfile>,
unsigned lineNumber, const String& sourceURL);

Remove these methods, they don't even have implementation in this class.

> Source/WebCore/inspector/InspectorHeapProfilerAgent.h:85
> +    InspectorHeapProfilerAgent(InstrumentingAgents*,
InspectorCompositeState*, InjectedScriptManager*);

Why is it not private?

> Source/WebCore/inspector/InspectorHeapProfilerAgent.h:88
> +    typedef HashMap<unsigned, RefPtr<ScriptHeapSnapshot> > HeapSnapshotsMap;


IsToHeapSnapshotMap