Resources fetched from another origin should not appear in detail on the Performance Timeline. Instead, only the startTime and duration attributes should be populated. Optionally, hosts can supply a Timing-Allow-Origin header to override this behavior.
Created attachment 161925 [details]
Created attachment 170155 [details]
Comment on attachment 170155 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=170155&action=review
> +static bool passesTimingAllowCheck(const ResourceResponse& response, SecurityOrigin* securityOrigin)
I think this should happen in the PerformanceResourceTiming constructor. It's really only applicable to that class.
> + if (timingAllowOriginString != securityOrigin->toString())
The string may be a space separated list. We should check each item in it.
> +void Performance::addResourceTiming(const ResourceRequest& request, const ResourceResponse& response, double finishTime, Document* requestingDocument, SecurityOrigin* initiatorOrigin)
We don't need to pass in the origin. You can get it from requestingDocument->securityOrigin().
> + bool allowTiming = WebCore::SecurityOrigin::create(response.url())->equal(initiatorOrigin) || passesTimingAllowCheck(response, initiatorOrigin);
allowTiming -> shouldReportDetails
> + static PassRefPtr<PerformanceResourceTiming> create(const ResourceRequest& request, const ResourceResponse& response, double finishTime, Document* requestingDocument, bool isAllowTiming)
isAllowTiming -> shouldReportDetails
> + PerformanceResourceTiming(const ResourceRequest&, const ResourceResponse&, double finishTime, Document*, bool);
This needs a name. You can only omit the name if it's clear what it is.
> + requestUrl += '?origin=http://' + pageOrigin;
We should have a test where it's allowed by this parameter.
(In reply to comment #3)
> > LayoutTests/http/tests/w3c/webperf/submission/Intel/resource-timing/test_resource_timing_timing_allow_cross_origin_resource_request.html:51
> > + requestUrl += '?origin=http://' + pageOrigin;
> We should have a test where it's allowed by this parameter.
Sorry, I misread the code. I saw the 0's but didn't notice the greater_than. I think we're good here.
Created attachment 177042 [details]
In the interest of getting this landed soon, I made the suggestions. Someone else (Tony?) will have to review it still.
Comment on attachment 177042 [details]
Clearing flags on attachment: 177042
Committed r136329: <http://trac.webkit.org/changeset/136329>
All reviewed patches have been landed. Closing bug.