http://trac.webkit.org/changeset/57535/ brought this into Safari. For now those layout tests: http/tests/xmlhttprequest/origin-whitelisting-all.html http/tests/xmlhttprequest/origin-whitelisting-exact-match.html http/tests/xmlhttprequest/origin-whitelisting-https.html http/tests/xmlhttprequest/origin-whitelisting-ip-addresses-with- subdomains.html http/tests/xmlhttprequest/origin-whitelisting-ip-addresses.html http/tests/xmlhttprequest/origin-whitelisting-subdomains.html http/tests/security/local-image-from-remote-whitelisted.html http/tests/xmlhttprequest/origin-whitelisting-removal.html are skipped for Chromium.
https://bugs.webkit.org/show_bug.cgi?id=37577 modified Chromium's test_expectations to skip the tests.
This simply looks like a work item for Chromium's DRT/test_shell. The work involved would be to expose this through the Chromium WebKit API and then use it in test_shell. Similar to what was done for Windows and OSX in https://bugs.webkit.org/attachment.cgi?id=53259&action=prettypatch Anton, perhaps you could tackle this as a clean up task?
(In reply to comment #2) > This simply looks like a work item for Chromium's DRT/test_shell. > > The work involved would be to expose this through the Chromium WebKit API and > then use it in test_shell. Similar to what was done for Windows and OSX in > https://bugs.webkit.org/attachment.cgi?id=53259&action=prettypatch > > Anton, perhaps you could tackle this as a clean up task? David, I'll try to implement it, but probably somewhat later. Or you think this is urgent?
(In reply to comment #3) > David, I'll try to implement it, but probably somewhat later. Or you think > this is urgent? Not urgent.
Some more updates. There were two relevant changes: http://trac.webkit.org/changeset/57535 renamed two methods and http://trac.webkit.org/changeset/57537 which added another one. My current plan would be to add new method into WebKit/WebKit/chromium and then submit a change to Chromium's test_shell which would both add new method and rename two renamed ones. After that we probably need two-sided patch to rename methods in Chromium's bridge API. Does it sound sane?
Created attachment 54566 [details] Patch
(In reply to comment #6) > Created an attachment (id=54566) [details] > Patch Name---unwhitelist....---is rather poor, but ideally we should move to add/remove as in SecurityPolicy.
Adding Darin since this is a public api change.
Comment on attachment 54566 [details] Patch WebKit/chromium/src/WebSecurityPolicy.cpp:69 + void WebSecurityPolicy::unwhiteListAccessFromOrigin(const WebURL& sourceOrigin, Why not use the same name as the method on SecurityOrigin? I have the same question about whiteListAccessFromOrigin. Oh, I see... in r57535, SecurityOrigin::whiteListAccessFromOrigin was renamed addOriginAccessWhitelistEntry :-( How about writing a patch that switches us over to the same method names as SecurityOrigin so that we can avoid the awkward sounding "unwhiteListFoo" method name?
I recommend doing two WebKit patches here: first patch adds the new method names, and the second patch removes the old method name. The chromium patch that you need to do to call the newly added "remove" function can also include a change to call the new form of the "add" function. Do that step before you submit the second WebKit patch.
Created attachment 54707 [details] Patch
(In reply to comment #10) > I recommend doing two WebKit patches here: first patch adds the new method > names, and the second patch removes the old method name. The chromium patch > that you need to do to call the newly added "remove" function can also include > a change to call the new form of the "add" function. Do that step before you > submit the second WebKit patch. Sure, Darin, and thanks a lot for comments. Please notice that there is almost invisible change in resetOriginAccessWhiteLists name: White<L>ists -> White<l>ists. May you have another look?
Comment on attachment 54707 [details] Patch > +// To be removed when Chromium's test_shell has proper references. > +void WebSecurityPolicy::whiteListAccessFromOrigin( Please add DEPRECATED to comments like the above, so that it is easier to grep for these dead bits. Otherwise R=me
Created attachment 54712 [details] Adding DEPRECATED tags
(In reply to comment #13) > (From update of attachment 54707 [details]) > > +// To be removed when Chromium's test_shell has proper references. > > +void WebSecurityPolicy::whiteListAccessFromOrigin( > > Please add DEPRECATED to comments like the above, so that > it is easier to grep for these dead bits. > > Otherwise R=me Thanks, Darin. Could you r+ new patch if it's fine---I'd rather to use commit-queue as this patch is not urgent.
Rather than make Darin re-review it if this is something you would submit given Darin's r+, then you should upload the patch with the reviewed lines fixed as if you were about to check it in and then just mark the patch as cq+ (and no r? or r+, etc. -- just leave that blank). The commit queue should handle that then.
Comment on attachment 54712 [details] Adding DEPRECATED tags See comment above.
Created attachment 54714 [details] Same as previous, to bypass r+
(In reply to comment #17) > (From update of attachment 54712 [details]) > See comment above. Thanks a lot, David. Trying this approach. Sorry for noise.
Created attachment 54715 [details] Now with correct Reviewed by
Comment on attachment 54715 [details] Now with correct Reviewed by Clearing flags on attachment: 54715 Committed r58530: <http://trac.webkit.org/changeset/58530>
All reviewed patches have been landed. Closing bug.
Created attachment 55247 [details] Remove deprecated methods
Attachment 55247 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/2231020
Created attachment 55248 [details] Adding DRT files
Thanks a lot, Darin.
I think this patch i supposed to be landed. So reopening the bug so the commit-queue will see it.
Comment on attachment 55248 [details] Adding DRT files Rejecting patch 55248 from commit-queue. Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Darin Fisher', u'--force']" exit_code: 1 Last 500 characters of output: atching file WebKitTools/DumpRenderTree/chromium/LayoutTestController.cpp Hunk #1 FAILED at 113. Hunk #2 FAILED at 469. Hunk #3 succeeded at 1109 with fuzz 1 (offset 56 lines). 2 out of 3 hunks FAILED -- saving rejects to file WebKitTools/DumpRenderTree/chromium/LayoutTestController.cpp.rej patching file WebKitTools/DumpRenderTree/chromium/LayoutTestController.h Hunk #1 FAILED at 230. 1 out of 1 hunk FAILED -- saving rejects to file WebKitTools/DumpRenderTree/chromium/LayoutTestController.h.rej Full output: http://webkit-commit-queue.appspot.com/results/2281052
Created attachment 56063 [details] Resolve a conflict
Comment on attachment 56063 [details] Resolve a conflict as cq got stuck, someone else partially fixed the issue this patch was supposed to fix. Merge was needed. cq+ it again.
Comment on attachment 56063 [details] Resolve a conflict Clearing flags on attachment: 56063 Committed r59512: <http://trac.webkit.org/changeset/59512>
Created attachment 56698 [details] Patch
And the last patch, hopefully. Try bot results: http://codereview.chromium.org/2104016
Comment on attachment 56698 [details] Patch Clearing flags on attachment: 56698 Committed r60033: <http://trac.webkit.org/changeset/60033>
Thanks a lot, Darin. Now that should be closed for the last time.