[Webkit-unassigned] [Bug 58652] Web Inspector: Background network events collection - add GUI to Inspector

[bugzilla-daemon at webkit.org]

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





--- Comment #6 from Sergey Vorobyev <sergeyvorobyev at google.com>  2011-04-29 06:51:52 PST ---
(From update of attachment 91650)
View in context: https://bugs.webkit.org/attachment.cgi?id=91650&action=review

>> LayoutTests/http/tests/inspector/network/network-open-load-reopen.html:6
>> +function reopenFrontend() {
> 
> Please move this function to inspector-test.js since it's shared by at least two tests.

Done.

>> LayoutTests/http/tests/inspector/network/network-open-load-reopen.html:35
>> +            InspectorTest.evaluateInPage("reopenFrontend()");
> 
> You might start reopening front-end before the resources have loaded.

Done. Add image.onload

>> LayoutTests/http/tests/inspector/network/network-open-load-reopen.html:40
>> +            for (i = 0; i < resourcesCount; i++) {
> 
> How can you be sure that all the images have already loaded?

Done, I think (same as previous)

>> Source/WebCore/inspector/Inspector.json:451
>> +                "name": "switchBackgroundEventsCollectionTo",
> 
> Should be setBackgroundEventCollectionEnabled(true/false).

Done.

>> Source/WebCore/inspector/Inspector.json:458
>> +                "name": "backgroundEventsCollectionEnabled",
> 
> getBackgroundEventCollectionState

Done.

>> Source/WebCore/inspector/InspectorResourceAgent.cpp:94
>> +        disable(0);
> 
> The contract is that ErrorString* is non-0.

ok, done

>> Source/WebCore/inspector/InspectorResourceAgent.cpp:-186
>> -        ErrorString error;
> 
> Please revert this.

done.

>> Source/WebCore/inspector/InspectorResourceAgent.cpp:351
>> +        InspectorFrontendChannel* client = m_frontend->getInspectorFrontendChannel();
> 
> Please put an ASSERT(m_frontend)

done

>> Source/WebCore/inspector/InspectorResourceAgent.cpp:355
>> +        // Take out Message Proxy from reveiver chain.
> 
> reveiver -> receiver

done

>> Source/WebCore/inspector/InspectorResourceAgent.h:108
>> +    Frame* frameForId(const String& frameId);
> 
> Should be removed?

yes, thanks

>> Source/WebCore/inspector/front-end/NetworkPanel.js:603
>> +        var self = this;
> 
> Use eventCollectionEnabled.bind(this) instead(like we do in other places).

done

>> Source/WebCore/inspector/front-end/NetworkPanel.js:604
>> +        function callbackEventsCollectionEnabled(error, enabled)
> 
> eventCollectionEnabled

done

>> Source/WebCore/inspector/front-end/NetworkPanel.js:610
>> +        this._backgroundEventsCollectionToggle.addEventListener("click", this._toggleBackgroundEventsCollection.bind(this), false);
> 
> I believe we should first try adding context menu item instead of the status bar item. We can move it to the status bar later when the feature will be tested.

Ok, done.

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