Closed Bug 1139953 Opened 5 years ago Closed 4 years ago
Accept cookie dialog shown in private window when e10s enabled
STR: - Enable e10s - Set network.cookie.lifetimePolicy=1 - Open private browsing window - Load a page which sets a cookie Result: The accept cookie dialog is shown Expected result: The dialog should not be shown I discovered this while trying to get privatebrowsing tests to run in e10s. The expected functionality is tested by browser_privatebrowsing_cookieacceptdialog.js and after making the test e10s compatible it failed due to this bug in privatebrowsing. I'll put the e10s ready test up for review as part of Bug 1132566 soon.  e.g.: browser/components/privatebrowsing/test/browser/browser_privatebrowsing_cookieacceptdialog.html  cookieAcceptDialog.xul
Assignee: nobody → mrbkap
The comment added in this patch tells the story here. We calculate everything, pass it through to nsICookieService from CookieServiceParent and then ignore it to check the channel (which we don't pass in). As far as I can tell, we end up doing some of these checks twice (including the third party check, though if we fail that, then we return early and don't ask the channel about it). The only bit that we need to pass is the aIsPrivate bit, so in true Gecko fashion, I created a dummy object to pass the state through. This code is very similar to nsHTMLDocument::CreateDummyChannelForCookies but is different enough than I couldn't really share any code.
Comment on attachment 8635545 [details] [diff] [review] Patch v1 Josh, can you review this? If not, I'll ask jduell for review. https://treeherder.mozilla.org/#/jobs?repo=try&revision=e58eaf4e29dd
Attachment #8635545 - Flags: review?(josh)
Comment on attachment 8635545 [details] [diff] [review] Patch v1 Review of attachment 8635545 [details] [diff] [review]: ----------------------------------------------------------------- This is not the first time we've resorted to this, and it probably won't be the last.
Attachment #8635545 - Flags: review?(josh) → review+
This landing appears to be causing a smoke test blocker bug 1186920. We may need this backed out.
Blake can we get this backed out for firefox OS until patched? see Francisco's comment in bug 1186920: "In contacts we use a cookie to get/store the value for ordering the contacts list. Is one of the first things that we do, it's in the critical path, and seems that's the reason why the app is failing to start."
I checked in a patch to fix b2g (see bug 1186920).
You need to log in before you can comment on or make changes to this bug.