[webkit-reviews] review granted: [Bug 132574] clean up ResourceLoadTiming : [Attachment 231298] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 132574: clean up ResourceLoadTiming
https://bugs.webkit.org/show_bug.cgi?id=132574

Attachment 231298: Patch
https://bugs.webkit.org/attachment.cgi?id=231298&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=231298&action=review


>
Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelega
te.mm:224
> +#if ENABLE(WEB_TIMING)

This is all needed in WebCoreResourceHandleAsDelegate.mm, too. There is no
reason to have it work incorrectly in WebKitLegacy.

>
Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelega
te.mm:226
> +	   NSDictionary* timingData = [connection _timingData];
> +	   if (timingData) {

Style nit: star goes to the other side for Objective-C, and you could combine
the definition with if:

    if (NSDictionary *timingData = [connection _timingData])
	...

>
Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelega
te.mm:228
> +	       ResourceLoadTiming* timing =
resourceResponse.resourceLoadTiming();

It seems strange to see a pointer variable used without a null check, or any
obvious reason why it can't be null. Can the function return a reference?