WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
132917
remove duplicate code in web timing
https://bugs.webkit.org/show_bug.cgi?id=132917
Summary
remove duplicate code in web timing
Alex Christensen
Reported
2014-05-14 11:12:01 PDT
I added some duplicate code copying timing data from the NSURLConnection to the ResourceLoadTiming in
https://bugs.webkit.org/show_bug.cgi?id=132574
and I'm going to move that code to ResourceHandle.
Attachments
Patch
(10.16 KB, patch)
2014-05-14 11:15 PDT
,
Alex Christensen
ap
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Alex Christensen
Comment 1
2014-05-14 11:15:48 PDT
Created
attachment 231459
[details]
Patch
Alexey Proskuryakov
Comment 2
2014-05-14 11:22:36 PDT
Comment on
attachment 231459
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=231459&action=review
> Source/WebCore/platform/network/ResourceHandle.h:33 > +#include "ResourceLoadTiming.h"
Please don't include it, just forward declare.
> Source/WebCore/platform/network/ResourceHandle.h:134 > + static void transferTimingData(NSURLConnection*, ResourceLoadTiming&);
I'd say "get", not "transfer".
> Source/WebCore/platform/network/mac/WebCoreResourceHandleAsDelegate.mm:175 > + ResourceHandle::transferTimingData(connection, resourceResponse.resourceLoadTiming());
This is not what I was thinking of - I think that a nicer way to implement this would be to call into ResourceHandle here, which would then read the timing data, and call the client. Duplicating even the ResourceHandle::transferTimingData still leaves the potential for us to forget about it in future reimplementations. It's OK to land as is though, the patch is an improvement.
Alex Christensen
Comment 3
2014-05-14 11:30:35 PDT
(In reply to
comment #2
)
> Please don't include it, just forward declare.
done
> I'd say "get", not "transfer".
done Committed with these changes to
http://trac.webkit.org/changeset/168849
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug