Some tests don't pass when running with the network process enabled
Related to this, NetworkProcess picks up cookie accept policy from Safari on Mac, so certain tests fail when the Safari preference is not the default one. I have a fix for both issues.
Created attachment 248468 [details] proposed fix
I should probably remove this Gtk expectation, too. LayoutTests/platform/gtk/TestExpectations:webkit.org/b/138687 http/tests/xmlhttprequest/cross-origin-cookie-storage.html [ Skip ]
Comment on attachment 248468 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=248468&action=review Thanks ap!, this looks good to me, and I confirm this fixes http/tests/xmlhttprequest/cross-origin-cookie-storage.html in GTK+ port. This changes WebCookieManagerProxy.cpp, so I guess I can't r+ this. > Tools/WebKitTestRunner/TestInvocation.cpp:652 > + // FIXME: This doesn't synchronously update the policy in WebProcess. This shouldn't affect tests much, because most loading is in NetworkProcess. I don't get this comment, the message is also async for the network process.
Comment on attachment 248468 [details] proposed fix Oops, sorry, I removed the flag :-P
Actually, you can r+ it - when the patch author is a WebKit2 owner, anyone can r+. > I don't get this comment, the message is also async for the network process. Good point, they are both affected. I was thinking of WebProcess primarily because it is blocked in a sync message send, and then continues parsing without an opportunity to handle the message. I can re-word the comment as: // FIXME: This updates the policy in WebProcess and in NetworkProcess asynchronously, which might break some tests' expectations.
(In reply to comment #6) > Actually, you can r+ it - when the patch author is a WebKit2 owner, anyone > can r+. Ah, I didn't know that, r=me then. > > I don't get this comment, the message is also async for the network process. > > Good point, they are both affected. I was thinking of WebProcess primarily > because it is blocked in a sync message send, and then continues parsing > without an opportunity to handle the message. > > I can re-word the comment as: > > // FIXME: This updates the policy in WebProcess and in NetworkProcess > asynchronously, which might break some tests' expectations. That makes more sense :-)
Comment on attachment 248468 [details] proposed fix Please, remember to unskip the GTK+ test too.
Created attachment 248523 [details] patch for landing
Comment on attachment 248523 [details] patch for landing Rejecting attachment 248523 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 248523, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit /Volumes/Data/EWS/WebKit/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://webkit-queues.appspot.com/results/5987861060911104
Created attachment 248525 [details] patch for landing Annoying. Not sure if we actually have a policy like that.
Comment on attachment 248525 [details] patch for landing Clearing flags on attachment: 248525 Committed r181446: <http://trac.webkit.org/changeset/181446>
All reviewed patches have been landed. Closing bug.