TypeError: ContextualIdentityService.getIdentityFromId(...) is undefined

RESOLVED DUPLICATE of bug 1337964

Status

()

P1
normal
RESOLVED DUPLICATE of bug 1337964
2 years ago
2 years ago

People

(Reporter: kjozwiak, Assigned: baku)

Tracking

(Blocks: 1 bug)

53 Branch
Points:
---

Firefox Tracking Flags

(firefox50 unaffected, firefox51 unaffected, firefox52 affected, firefox53 affected)

Details

(Whiteboard: [userContextId][domsecurity-backlog])

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
Created attachment 8813873 [details]
issue.png

When you're removing containers or clearing cookies, you'll sometimes get into a situation where the cookie manager stops working under about:preferences#privacy and will appear completely blank. You'll also receive a "TypeError: ContextualIdentityService.getIdentityFromId(...) is undefined" error message under the browser console.

Platforms Used:
================

* macOS 10.12.1 x64 - reproduced on both fx53 and fx52
* Win 10 x64 - reproduced on both fx53 and fx52
* Ubuntu 16.04 x64 - reproduced on both fx53 and fx52

Browser Console Error:
======================

TypeError: ContextualIdentityService.getIdentityFromId(...) is undefined cookies.js:86:15
gCookiesWindow._isPrivateCookie chrome://browser/content/preferences/cookies.js:86:15
gCookiesWindow._loadCookies chrome://browser/content/preferences/cookies.js:496:13
gCookiesWindow._populateList chrome://browser/content/preferences/cookies.js:50:5
gCookiesWindow.init chrome://browser/content/preferences/cookies.js:33:5
onload chrome://browser/content/preferences/cookies.xul:1:1

STR:
====

* install and launch the latest version of fx
* open a new personal container via File -> New Container Tab
* load gmail.com within the personal container
* open a new tab and remove the personal container via about:preferences#containers
* ensure that the personal container is the only tab that's opened
* restart fx and click on "Restore Previous Session"
* open a new tab and visit the cookie manager via about:preferences#privacy
(Reporter)

Updated

2 years ago
Has STR: --- → yes
status-firefox50: --- → unaffected
status-firefox51: --- → unaffected
status-firefox52: --- → affected

Updated

2 years ago
Priority: -- → P2
(Reporter)

Comment 1

2 years ago
STR:

* open a freshly installed m-c
* open a personal container via "File -> New Container Tab"
* load facebook.com inside the personal container and close the remaining tabs
* open about:preferences#containers in a new tab and remove the personal container
* once the container has been removed, click on "Privacy" on the left hand side
* click on "remove individual cookies"

Short video of the issue being reproduced:
* https://youtu.be/kOHydArPJAE
(Assignee)

Updated

2 years ago
Assignee: nobody → amarchesini
(Assignee)

Comment 2

2 years ago
I cannot reproduce it with the latest nightly. Do you mind to check if it's already fixed?
Flags: needinfo?(kjozwiak)
(Reporter)

Comment 3

2 years ago
As we discussed in the all hands, I went through the STR from comment#1 and could still reproduce the issue using the latest debug builds [1]. Using a non-debug build on my personal Windows desktop, I managed to reproduce this on a very old profile. The concerning part is that the issue was persistent even after restarting the browser several times. Usually when I reproduce this bug and restart the browser, the cookie manager is restored to it's correct behaviour.

[1] Debug builds being used:

* fx53.0a1, buildid: 20161214214334, changeset: b1ab720c6d3e (Ubuntu 16.04.1 LTS)
* fx53.0a1, buildid: 20161214214711, changeset: b1ab720c6d3e (Win 10 Pro x64)
* fx53.0a1, buildid: 20161214174132, changeset: 18b5a7a5d833 (macOS 10.12.2 x64)
Flags: needinfo?(kjozwiak)
(Assignee)

Comment 4

2 years ago
I'm not able to reproduce it. But maybe we can find the reason of this bug in the log. Kamil, can you please upload the log of FF when this issue appears?
Flags: needinfo?(kjozwiak)
(Reporter)

Comment 5

2 years ago
I went through the STR using a debug build and attached the output from the terminal [1]. The original JS error occurs on line 151 [2]. There's also a bunch of assertions that occurred this time around but I'm not sure if they're related to this particular issue as they don't happen all the time.

Baku, let me know if there's another log that I can provide. I'm not sure if this is the log that you wanted.

[1] https://gist.github.com/kjozwiak/a2d82e2ffe36ab0a6ab4a61a75590a6c#file-terminallog-txt
[2] https://gist.github.com/kjozwiak/a2d82e2ffe36ab0a6ab4a61a75590a6c#file-terminallog-txt-L151
Flags: needinfo?(kjozwiak)
(Assignee)

Comment 6

2 years ago
Created attachment 8820387 [details] [diff] [review]
cookie.patch

When a container is deleted, nothing prevents other components to ask for that identity in the meantime.
Attachment #8820387 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Comment 7

2 years ago
Comment on attachment 8820387 [details] [diff] [review]
cookie.patch

I know what the _real_ problem is. When a container tab is opened and the user wants to delete such container, we close that tab. Then we propagate a "clear-origin-attributes-data".

Unfortunately, it's possible that we have an active http connection and this sets a cookie _after_ the dispatching of "clear-origin-attributes-data".

When this happens, we have a cookie with a UCI, without having that container ID.
Attachment #8820387 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Comment 8

2 years ago
Comment on attachment 8820387 [details] [diff] [review]
cookie.patch

I haven't found any better way to deal with this problem.
Attachment #8820387 - Flags: review?(gijskruitbosch+bugs)

Comment 9

2 years ago
Comment on attachment 8820387 [details] [diff] [review]
cookie.patch

Review of attachment 8820387 [details] [diff] [review]:
-----------------------------------------------------------------

I think this produces the wrong result for the name of the function (is this cookie private) in the most common case, as we don't yet support customizable private containers.

More generally, we should kill open http requests, service workers, etc. when a container goes away. That feels like a larger problem, and this feels very wallpapery. Is there a way to avoid that?

::: browser/components/preferences/cookies.js
@@ +85,5 @@
>        }
> +      let identity = ContextualIdentityService.getIdentityFromId(userContextId);
> +      // If the identity doesn't exist, probably it's because it has been
> +      // deleted but this cookie is still around.
> +      return !identity || !identity.public;

Why should the identity not existing imply the cookie is private? Shouldn't it be marked public?
Attachment #8820387 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Comment 10

2 years ago
it seems that this issue happens only with e10s so definitely it's a timing issue: the tab is not completely closed when removeTab() returns.

Comment 11

2 years ago
baku, are you still working on this or shoudl we unassign?  In your opinion, how important is this?
Flags: needinfo?(amarchesini)

Updated

2 years ago
See Also: → bug 1337964
(Reporter)

Comment 12

2 years ago
(In reply to Tanvi Vyas - not reading bugmail, PM me [:tanvi] from comment #11)
> baku, are you still working on this or shoudl we unassign?  In your opinion,
> how important is this?

I personally think we should fix this whenever possible. Unfortunately this bug is in the release version of fx as well, which is affecting the test pilot as well. I've managed to reproduce this bug a few times while going through the test pilot [1][2]. It's pretty bad regression as the user will lose functionality, and restarting fx doesn't always fix the problem.

From the above "See Also" from Gijs, it looks like people are running into this as well.

[1] https://github.com/mozilla/testpilot-containers/issues/172
[2] https://github.com/mozilla/testpilot-containers/issues/103
(Reporter)

Updated

2 years ago
Priority: P2 → P1
(Assignee)

Comment 13

2 years ago
I have submitted a patch for bug 1337964.
Flags: needinfo?(amarchesini)
(Assignee)

Comment 14

2 years ago
Kamil, are you able to reproduce this issue after bug 1337964?
Flags: needinfo?(kjozwiak)
(Reporter)

Comment 15

2 years ago
(In reply to Andrea Marchesini [:baku] from comment #14)
> Kamil, are you able to reproduce this issue after bug 1337964?

Nope, I can't reproduce this anymore after bug#1337964 landed. I went through the following test cases:

Reproduced the original issue using STR from comment#1 using the following build:
* fx54.0a1, buildid: 20170214030231, changeset: 195049fabb7a
** https://archive.mozilla.org/pub/firefox/nightly/2017/02/2017-02-14-03-02-31-mozilla-central/

Went through verification using the STR from comment#1 using the following build:
* fx55.0a1, buildid: 20170321030211, changeset: 5fe5dcf1c10a
* fx54.0a2, buildid: 20170321004003, changeset: 0496e366a8f4

Platforms Used:

* macOS 10.12.3 x64 - PASSED
* Win 10 Pro x64 VM - PASSED
* Ubuntu 16.04.2 LTS x64 VM - PASSED
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Flags: needinfo?(kjozwiak)
Resolution: --- → DUPLICATE
Duplicate of bug: 1337964
You need to log in before you can comment on or make changes to this bug.