As promised in https://bugs.webkit.org/show_bug.cgi?id=132574 I'm using references for ResourceLoadTiming, which is instanced in PerformanceResourceTiming and ResourceResponseBase. This avoids unnecessary null checks and reference counting, and it makes the code nicer to read.
Created attachment 231348 [details] Patch
Created attachment 231382 [details] Patch
Created attachment 231386 [details] Patch
Created attachment 231403 [details] Patch
The fourth patch tried to get rid of my use of mutable in ResourceResponseBase.h, but it doesn't work nicely with the way Soup uses ResourceLoadTiming. I think the third patch is the way to go. It uses mutable, but there are lots of mutable things in ResourceResponseBase.
Comment on attachment 231386 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231386&action=review Is there a reason why you can't just have an std::unique_ptr reference to the ResourceLoadTiming object? Maybe it doesn't matter. r- because of the assignment operator. > Source/WebCore/platform/network/ResourceLoadTiming.h:65 > + void operator=(const ResourceLoadTiming& other) > + { > + domainLookupStart = other.domainLookupStart; > + domainLookupEnd = other.domainLookupEnd; > + connectStart = other.connectStart; > + connectEnd = other.connectEnd; > + requestStart = other.requestStart; > + responseStart = other.responseStart; > + secureConnectionStart = other.secureConnectionStart; > + } This is not a correct assignment operator, it needs to return *this. > Source/WebCore/platform/network/ResourceResponseBase.cpp:555 > lazyInit(CommonAndUncommonFields); Do we need to call this now?
Created attachment 231406 [details] Patch
> Is there a reason why you can't just have an std::unique_ptr reference to the ResourceLoadTiming object? It seems nice to avoid a separate allocation, given that nearly(?) all of the responses will have the timing data.
Comment on attachment 231406 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231406&action=review And the downside is of course that touching ResourceTiming header will make half of WebCore rebuild, because half of WebCore indirectly includes ResourceResponse.h. > Source/WebCore/platform/network/mac/WebCoreResourceHandleAsDelegate.mm:197 > + timing.domainLookupStart = domainLookupStart <= 0.0 ? -1 : (domainLookupStart - referenceStart) * 1000; I don't think that we ever need to compare to 0.0, can it be simply 0?
(In reply to comment #9) > I don't think that we ever need to compare to 0.0, can it be simply 0? I'll do this when I refactor the duplicate code.
http://trac.webkit.org/changeset/168845