[webkit-reviews] review denied: [Bug 166477] Web Inspector: after inspecting an image, selecting the main resource should show an image preview : [Attachment 306591] Patch

[bugzilla-daemon at webkit.org]

Joseph Pecoraro <joepeck at webkit.org> has denied Matt Baker
<mattbaker at apple.com>'s request for review:
Bug 166477: Web Inspector: after inspecting an image, selecting the main
resource should show an image preview
https://bugs.webkit.org/show_bug.cgi?id=166477

Attachment 306591: Patch

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




--- Comment #6 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 306591
  --> https://bugs.webkit.org/attachment.cgi?id=306591
Patch

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

I'm going to r- because this patch probably no longer applies.

>> Source/WebCore/inspector/InspectorNetworkAgent.cpp:383
>> +	if (type != newType && (type != InspectorPageAgent::OtherResource &&
newType != InspectorPageAgent::DocumentResource) && newType !=
InspectorPageAgent::XHRResource && newType !=
InspectorPageAgent::OtherResource)
> 
> This feels wrong. I'm not sure what type transitions are allowed here. Will
investigate.

I agree, this is confusing. It needs some kind of justification / explanation.
Perhaps writings the condition differently would make it clearer?

I think there are other places in the code that say "If this is a MainResource
assume its a DocumentResource". Maybe that is the ultimate problem that should
be addressed?