[Webkit-unassigned] [Bug 58064] Web Inspector: Network events don't preserves, when inspector frontend closed and open again

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=58064


Yury Semikhatsky <yurys at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #89009|review?                     |review-
               Flag|                            |




--- Comment #16 from Yury Semikhatsky <yurys at chromium.org>  2011-04-14 04:44:57 PST ---
(From update of attachment 89009)
View in context: https://bugs.webkit.org/attachment.cgi?id=89009&action=review

> Source/WebCore/ChangeLog:9
> +        No new tests. (OOPS!)

Would be really nice to have a test for this change.

> Source/WebCore/inspector/EventsCollector.cpp:38
> +{ }

style: closing bracket should go on a new line.

> Source/WebCore/inspector/EventsCollector.cpp:42
> +    totalLength += (int)message.length();

we don't use c-style casts in WebCore code. You could use size_t as m_totalLength type.

> Source/WebCore/inspector/EventsCollector.h:42
> +    void collect(const String& message);

I'd rename it to addEvent

> Source/WebCore/inspector/EventsCollector.h:43
> +    void pushData(InspectorFrontendChannel*);

Consider renaming pushData to sendCollectedEvents?

> Source/WebCore/inspector/EventsCollector.h:45
> +    static const int CAPACITY = 1048576; // 1 Mb

WebCore constant names should be in camelCase, like maxCapacity. Also please use 1024*1024 instead of the number, compiler will do this evaluation for you.

> Source/WebCore/inspector/EventsCollector.h:46
> +    int totalLength;

style: private fields should start with m_ prefix(http://www.webkit.org/coding/coding-style.html)

> Source/WebCore/inspector/InspectorFrontendProxy.h:40
> +    InspectorFrontendProxy(EventsCollector*);

Constructors with single argument should be marked as "explicit".

> Source/WebCore/inspector/InspectorResourceAgent.cpp:549
> +    m_frontend = new InspectorFrontend::Network(m_inspectorFrontendProxy.get());

You're leaking the Network object here. Instead it should be owned by the agent and created only when background resource tracking is enabled.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.