Bug 175746

Summary: [Beacon] Content extensions should be able to intercept Beacon / Ping redirects
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit2Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, beidson, buildbot, commit-queue, dbates, ggaren, japhet, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 175756    
Bug Blocks: 147885    
Attachments:
Description Flags
WIP Patch
none
WIP Patch
none
WIP Patch
none
WIP Patch
none
WIP Patch
none
WIP Patch
none
Patch none

Chris Dumez
Reported 2017-08-18 17:01:36 PDT
Content extensions should be able to intercept Beacon / Ping redirects.
Attachments
WIP Patch (9.50 KB, patch)
2017-08-21 11:34 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
WIP Patch (13.77 KB, patch)
2017-08-21 12:20 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
WIP Patch (13.81 KB, patch)
2017-08-21 12:59 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
WIP Patch (19.40 KB, patch)
2017-08-21 14:24 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
WIP Patch (19.88 KB, patch)
2017-08-21 14:26 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
WIP Patch (20.97 KB, patch)
2017-08-21 14:48 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (26.71 KB, patch)
2017-08-21 15:28 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2017-08-18 17:01:51 PDT
<rdar://problem/33946050>
Chris Dumez
Comment 2 2017-08-21 11:34:12 PDT
Created attachment 318650 [details] WIP Patch
Chris Dumez
Comment 3 2017-08-21 12:20:18 PDT
Created attachment 318658 [details] WIP Patch
Chris Dumez
Comment 4 2017-08-21 12:59:50 PDT
Created attachment 318662 [details] WIP Patch
Chris Dumez
Comment 5 2017-08-21 14:24:57 PDT
Created attachment 318672 [details] WIP Patch
Chris Dumez
Comment 6 2017-08-21 14:26:34 PDT
Created attachment 318673 [details] WIP Patch
Chris Dumez
Comment 7 2017-08-21 14:48:49 PDT
Created attachment 318676 [details] WIP Patch
Chris Dumez
Comment 8 2017-08-21 15:28:32 PDT
Created attachment 318678 [details] Patch
Alex Christensen
Comment 9 2017-08-21 17:38:01 PDT
Comment on attachment 318678 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=318678&action=review > Source/WebKit/NetworkProcess/PingLoad.cpp:129 > completionHandler({ }); > + didFinish(ResourceError { String(), 0, currentRequest().url(), ASCIILiteral("Blocked by Content Security Policy"), ResourceError::Type::AccessControl }); I think it would be nicer if the didFinish calls were before the completionHandler calls.
Chris Dumez
Comment 10 2017-08-21 18:03:41 PDT
(In reply to Alex Christensen from comment #9) > Comment on attachment 318678 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=318678&action=review > > > Source/WebKit/NetworkProcess/PingLoad.cpp:129 > > completionHandler({ }); > > + didFinish(ResourceError { String(), 0, currentRequest().url(), ASCIILiteral("Blocked by Content Security Policy"), ResourceError::Type::AccessControl }); > > I think it would be nicer if the didFinish calls were before the > completionHandler calls. This is not possible because didFinish() calls delete this;
WebKit Commit Bot
Comment 11 2017-08-21 18:32:29 PDT
Comment on attachment 318678 [details] Patch Clearing flags on attachment: 318678 Committed r220996: <http://trac.webkit.org/changeset/220996>
WebKit Commit Bot
Comment 12 2017-08-21 18:32:30 PDT
All reviewed patches have been landed. Closing bug.
Geoffrey Garen
Comment 13 2017-08-22 10:19:15 PDT
Comment on attachment 318678 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=318678&action=review > Source/WebKit/NetworkProcess/PingLoad.cpp:324 > +// Returns true if we should block the load. This would be nicer as an enum class return value. Maybe just return status, and then the caller can check status.blockedLoad?
Chris Dumez
Comment 14 2017-08-22 10:26:37 PDT
(In reply to Geoffrey Garen from comment #13) > Comment on attachment 318678 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=318678&action=review > > > Source/WebKit/NetworkProcess/PingLoad.cpp:324 > > +// Returns true if we should block the load. > > This would be nicer as an enum class return value. Maybe just return status, > and then the caller can check status.blockedLoad? I copied the existing pattern but I agree. I'll clean this up in a follow-up.
Chris Dumez
Comment 15 2017-08-22 12:14:23 PDT
(In reply to Chris Dumez from comment #14) > (In reply to Geoffrey Garen from comment #13) > > Comment on attachment 318678 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=318678&action=review > > > > > Source/WebKit/NetworkProcess/PingLoad.cpp:324 > > > +// Returns true if we should block the load. > > > > This would be nicer as an enum class return value. Maybe just return status, > > and then the caller can check status.blockedLoad? > > I copied the existing pattern but I agree. I'll clean this up in a follow-up. Done via Bug 175834.
Note You need to log in before you can comment on or make changes to this bug.