[webkit-reviews] review granted: [Bug 184396] Web Inspector backend should get headers & cookies from network process separately from resource requests : [Attachment 338314] Patch

[bugzilla-daemon at webkit.org]

Brian Burg <bburg at apple.com> has granted youenn fablet <youennf at gmail.com>'s
request for review:
Bug 184396: Web Inspector backend should get headers & cookies from network
process separately from resource requests
https://bugs.webkit.org/show_bug.cgi?id=184396

Attachment 338314: Patch

https://bugs.webkit.org/attachment.cgi?id=338314&action=review




--- Comment #9 from Brian Burg <bburg at apple.com> ---
Comment on attachment 338314
  --> https://bugs.webkit.org/attachment.cgi?id=338314
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=338314&action=review

r=me

> Source/WebCore/inspector/agents/InspectorNetworkAgent.cpp:426
> +    return source != ResourceResponse::Source::MemoryCache && source !=
ResourceResponse::Source::ServiceWorker && source !=
ResourceResponse::Source::ApplicationCache && source !=
ResourceResponse::Source::MemoryCacheAfterValidation;

Nit; a switch would be easier to read & maintain.

> Source/WebCore/loader/LoaderStrategy.h:85
> +    virtual ResourceResponse responseFromResourceLoadIdentifier(uint64_t /*
resourceLoadIdentifier */);

Nit: no need to comment out the parameter names in the declaration.

> Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:279
> +	   // FIXME: All loaders should provide their origin if navigation mode
is cors/no-cors/same-origin.

I don't quite understand this part. On the assumption it's going to be used for
things other than Inspector, I'll let you determine the best way forward to
make it more accurate.