Private docshell tracking won't work with multiple content processes

RESOLVED FIXED in mozilla15

Status

()

Core
Document Navigation
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jdm, Assigned: jdm)

Tracking

Trunk
mozilla15
x86
Mac OS X
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
We probably need to implement a check for process type and send a message if it's in a content process. The tracking should happen in the chrome process instead.
(Assignee)

Updated

5 years ago
Blocks: 722857
(Assignee)

Comment 1

5 years ago
Proposal: DecreasePrivateDocShellCount checks the process type - if in the chrome process, it fires the observer notification. If in the content process, it sends an async message to chrome which indicates that all of that process' private docshells have died. The chrome process, on receiving this message, broadcasts an async query to all content processes, which respond with a boolean indicating whether any private contexts exist. If all processes report back with a negative, the chrome process triggers the last-pb-context-destroyed notification.

This could lead to some weird edge cases involving processes dying or being created while waiting for responses to the broadcast. That sounds less than appealing.
(Assignee)

Comment 2

5 years ago
We could simplify it by a push mechanism - when the count becomes nonzero, a content process sends a message to the chrome process notifying it of this fact, causing the content process actor to be placed in a list. When the count drops to zero, the content process sends a message, and the actor is removed from the list. When a process dies, it is removed from the list. At any time when the list becomes empty, the notification is dispatched to all content processes (and locally within the content process). This is a much easier system to manage.
(Assignee)

Comment 3

5 years ago
Created attachment 615628 [details] [diff] [review]
Make docshell privacy notifications work across multiple processes.

Sent to try to demonstrate that this works via the new test.
Attachment #615628 - Flags: review?(bzbarsky)
(Assignee)

Comment 4

5 years ago
The GetProcessType weirdness in nsDSURIContentListener is due to LockPref being unavailable in content processes and thus killing my IPC xpcshell test with an assertion.
(Assignee)

Updated

5 years ago
Whiteboard: [needs review]

Comment 5

5 years ago
Try run for f69bb2fc9baf is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=f69bb2fc9baf
Results (out of 15 total builds):
    exception: 14
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/josh@joshmatthews.net-f69bb2fc9baf

Comment 6

5 years ago
Try run for 573a0cc3c667 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=573a0cc3c667
Results (out of 15 total builds):
    exception: 14
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/josh@joshmatthews.net-573a0cc3c667

Comment 7

5 years ago
Try run for 10f93d5ee486 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=10f93d5ee486
Results (out of 25 total builds):
    success: 12
    warnings: 10
    failure: 3
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/josh@joshmatthews.net-10f93d5ee486
(Assignee)

Comment 8

5 years ago
Comment on attachment 615628 [details] [diff] [review]
Make docshell privacy notifications work across multiple processes.

Hold up, the test went orange on every platform on try.
Attachment #615628 - Flags: review?(bzbarsky)
(Assignee)

Comment 9

5 years ago
Created attachment 615756 [details] [diff] [review]
Make docshell privacy notifications work across multiple processes.
(Assignee)

Updated

5 years ago
Attachment #615628 - Attachment is obsolete: true

Comment 10

5 years ago
Try run for 1546ed1a6bf7 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=1546ed1a6bf7
Results (out of 29 total builds):
    exception: 4
    success: 15
    warnings: 10
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/josh@joshmatthews.net-1546ed1a6bf7

Comment 11

5 years ago
Try run for 35f4d5e28044 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=35f4d5e28044
Results (out of 3 total builds):
    exception: 2
    warnings: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/josh@joshmatthews.net-35f4d5e28044

Comment 12

5 years ago
Try run for 3375c1ae9d5f is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=3375c1ae9d5f
Results (out of 4 total builds):
    exception: 1
    success: 1
    warnings: 1
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/josh@joshmatthews.net-3375c1ae9d5f

Comment 13

5 years ago
Try run for c265a2f8cbc9 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=c265a2f8cbc9
Results (out of 3 total builds):
    exception: 1
    success: 1
    warnings: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/josh@joshmatthews.net-c265a2f8cbc9
(Assignee)

Comment 14

5 years ago
Created attachment 616817 [details] [diff] [review]
Make docshell privacy notifications work across multiple processes.

Now including a test that passes on all platforms unless otherwise-disabled.
Attachment #616817 - Flags: review?(bzbarsky)
(Assignee)

Updated

5 years ago
Attachment #615756 - Attachment is obsolete: true
Comment on attachment 616817 [details] [diff] [review]
Make docshell privacy notifications work across multiple processes.

r=me
Attachment #616817 - Flags: review?(bzbarsky) → review+
(Assignee)

Updated

5 years ago
Whiteboard: [needs review] → [needs landing]
(Assignee)

Updated

5 years ago
Assignee: nobody → josh
(Assignee)

Comment 16

5 years ago
Nevermind; something broke creating docshells in xpcshell tests since the last time I rebased.
Whiteboard: [needs landing]
(Assignee)

Comment 17

5 years ago
That was a lie; I just needed to rebuild.
Whiteboard: [needs landing]

Comment 18

5 years ago
Try run for ceb36014bc8e is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=ceb36014bc8e
Results (out of 13 total builds):
    success: 10
    warnings: 3
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/josh@joshmatthews.net-ceb36014bc8e
https://hg.mozilla.org/projects/birch/rev/f59333cbb1b2
Flags: in-testsuite+
Target Milestone: --- → mozilla15
Whiteboard: [needs landing]
https://hg.mozilla.org/mozilla-central/rev/f59333cbb1b2
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.