We need a test, it should be possible to use Windows APIs through ctyped to create a cookie for migration.

Did a try run just to ensure there's no unexpected failure. https://treeherder.mozilla.org/#/jobs?repo=try&revision=eb153a24754c

Comment on attachment 8592899 [details] [diff] [review] patch v1 Review of attachment 8592899 [details] [diff] [review]: ----------------------------------------------------------------- This is pretty amazing. If you get to it before me, please feel free to land together with the date patch I stuck in the other bug. Minor niggles and suggestions below. :-) ::: browser/components/migration/tests/unit/test_IE_cookies.js @@ +8,5 @@ > + > + const BOOL = ctypes.bool; > + const LPCTSTR = ctypes.char16_t.ptr; > + > + let wininet = ctypes.open("Wininet"); Can this throw and if so should we catch it and log a message so that if this ever breaks on try we have information? Say, if we start running this on win10 and for some reason it goes busted there? :-) @@ +35,5 @@ > + const COOKIE = { > + host: "mycookietest.com", > + name: "testcookie", > + value: "testvalue", > + expiry Wat. I didn't know this was possible in the new syntax, too. TIL. @@ +45,5 @@ > + > + // Create the persistent cookie in IE. > + let value = COOKIE.name + " = " + COOKIE.value +"; expires = " + > + COOKIE.expiry.toUTCString(); > + setIECookie(new URL("http://" + COOKIE.host).href, null, value); So dumb question... what does your API call do if this cookie already exists and is that going to blow up if the test runs a second time on the same machine? I'm assuming you've tested this... :-) Also, do I understand correctly that you declare this as returning a bool and then don't check the rv? Should we add a check for that just to be sure so that if this fails (returns false) we get that info and don't get inexplicable test failures? :-) @@ +65,5 @@ > + Assert.equal(foundCookie.expiry, Math.floor(COOKIE.expiry / 1000)); > +}); > + > +function run_test() { > + run_next_test() Is this necessary or something? Despite the add_task? That seems silly...

(In reply to :Gijs Kruitbosch from comment #2) > > + let wininet = ctypes.open("Wininet"); > > Can this throw and if so should we catch it and log a message so that if > this ever breaks on try we have information? Say, if we start running this > on win10 and for some reason it goes busted there? :-) The test would fail then, and we'd get the usual exception with line number. I'm not sure it's useful to catch it preemptively. > > + // Create the persistent cookie in IE. > > + let value = COOKIE.name + " = " + COOKIE.value +"; expires = " + > > + COOKIE.expiry.toUTCString(); > > + setIECookie(new URL("http://" + COOKIE.host).href, null, value); > > So dumb question... what does your API call do if this cookie already exists > and is that going to blow up if the test runs a second time on the same > machine? I'm assuming you've tested this... :-) I actually assumed the Windows API would overwrite it silently, as it would happen for a cookie set by the browser. I will test that, but I think any other behavior would not make sense. > Also, do I understand correctly that you declare this as returning a bool > and then don't check the rv? Should we add a check for that just to be sure > so that if this fails (returns false) we get that info and don't get > inexplicable test failures? :-) Yep, I forgot to check the rv. It would still fail, but better to be explicit. > @@ +65,5 @@ > > + Assert.equal(foundCookie.expiry, Math.floor(COOKIE.expiry / 1000)); > > +}); > > + > > +function run_test() { > > + run_next_test() > > Is this necessary or something? Despite the add_task? That seems silly... I didn't know bug 982852 was finally fixed 10 days ago... Good to know.

Nice work, thanks for getting this test in Marco!