[Webkit-unassigned] [Bug 61205] Web Inspector: Cache XHR content in backend, do not use initialContentSet for XHRs.

[bugzilla-daemon at webkit.org]

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





--- Comment #18 from Andrey Kosyakov <caseq at chromium.org>  2011-06-09 11:08:10 PST ---
(From update of attachment 96596)
View in context: https://bugs.webkit.org/attachment.cgi?id=96596&action=review

> LayoutTests/http/tests/inspector/network/network-xhr-async.html:5
> +function loadData()

nit: I think this is common enough to go to inspector-test or some other shared collection of test utilities.

> LayoutTests/http/tests/inspector/network/network-xhr-async.html:10
> +        if (xhr.readyState == XMLHttpRequest.DONE)

nit: ===

> LayoutTests/http/tests/inspector/network/network-xhr-async.html:13
> +    xhr.open('GET', 'resources/resource.php', true);

nit: we prefer double quotes.

> Source/WebCore/inspector/Inspector.json:493
> +                    { "name": "loaderId", "type": "string", "description": "Loader id of the resource to get content for." },

I don't think we need to require loaderId to fetch the resource content. Through the reset of ResourceAgent interface, we presume that resource identifier uniquely identifies the resource. I'm aware of that it's not so in case front-end preserves resource log during navigation, but this should be addressed on front-end rather than exposed in the back-end interface (front-end has loaderIds anyway).

> Source/WebCore/inspector/NetworkResourcesData.cpp:76
> +    if (!m_identifierToResourceDataMap.contains(identifier))

This appears redundant, just do a get() below and check for 0 (or use find).

> Source/WebCore/inspector/NetworkResourcesData.cpp:83
> +    if (!m_identifierToResourceDataMap.contains(identifier))

ditto.

> Source/WebCore/inspector/NetworkResourcesData.cpp:105
> +    if (!m_identifierToResourceDataMap.contains(identifier))

ditto.

> Source/WebCore/inspector/NetworkResourcesData.cpp:131
> +    m_identifierToResourceDataMap.clear();
> +
> +    end = preservedMap.end();
> +    for (it = preservedMap.begin(); it != end; ++it)
> +        m_identifierToResourceDataMap.set(it->first, it->second);
> +}

m_identifierToResourceDataMap.swap(preservedMap);

> Source/WebCore/inspector/NetworkResourcesData.h:53
> +        bool isContentDeleted;

nit: I don't quite like deleted here -- I'd suggest either isContentPurged, or reversing the meaning of the flag and calling it isContentAvailable.

> Source/WebCore/inspector/front-end/NetworkManager.js:53
> +        if (resource.identifier != null && resource.loaderId != null)

nit: resource.identifier && resource.loaderId

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