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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Apr 14 06:57:19 PDT 2011


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





--- Comment #17 from Sergey Vorobyev <sergeyvorobyev at google.com>  2011-04-14 06:57:19 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.

First commit would be without test and it will contain disable functionality
Next commit would contain tests and machinery for enable functionality

>> Source/WebCore/inspector/EventsCollector.cpp:38
>> +{ }
> 
> style: closing bracket should go on a new line.

done

>> 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.

done

>> Source/WebCore/inspector/EventsCollector.h:42
>> +    void collect(const String& message);
> 
> I'd rename it to addEvent

done

>> Source/WebCore/inspector/EventsCollector.h:43
>> +    void pushData(InspectorFrontendChannel*);
> 
> Consider renaming pushData to sendCollectedEvents?

It's much good, done

>> 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.

done

>> Source/WebCore/inspector/EventsCollector.h:46
>> +    int totalLength;
> 
> style: private fields should start with m_ prefix(http://www.webkit.org/coding/coding-style.html)

done.

>> Source/WebCore/inspector/InspectorFrontendProxy.h:40
>> +    InspectorFrontendProxy(EventsCollector*);
> 
> Constructors with single argument should be marked as "explicit".

done

>> 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.

done

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



More information about the webkit-unassigned mailing list