[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


Yury Semikhatsky <yurys at chromium.org> changed:

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




--- Comment #5 from Yury Semikhatsky <yurys at chromium.org>  2011-04-29 02:15:27 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.

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

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

> Source/WebCore/inspector/Inspector.json:451
> +                "name": "switchBackgroundEventsCollectionTo",

Should be setBackgroundEventCollectionEnabled(true/false).

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

getBackgroundEventCollectionState

> Source/WebCore/inspector/InspectorResourceAgent.cpp:94
> +        disable(0);

The contract is that ErrorString* is non-0.

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

Please revert this.

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

Please put an ASSERT(m_frontend)

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

reveiver -> receiver

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

Should be removed?

> Source/WebCore/inspector/front-end/NetworkPanel.js:603
> +        var self = this;

Use eventCollectionEnabled.bind(this) instead(like we do in other places).

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

eventCollectionEnabled

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

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