Last Comment Bug 729204 - Private docshell tracking won't work with multiple content processes
: Private docshell tracking won't work with multiple content processes
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Document Navigation (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla15
Assigned To: Josh Matthews [:jdm]
:
Mentors:
Depends on:
Blocks: PBnGen 722857
  Show dependency treegraph
 
Reported: 2012-02-21 10:52 PST by Josh Matthews [:jdm]
Modified: 2012-04-24 18:04 PDT (History)
5 users (show)
ehsan: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Make docshell privacy notifications work across multiple processes. (16.81 KB, patch)
2012-04-17 00:51 PDT, Josh Matthews [:jdm]
no flags Details | Diff | Splinter Review
Make docshell privacy notifications work across multiple processes. (17.66 KB, patch)
2012-04-17 09:56 PDT, Josh Matthews [:jdm]
no flags Details | Diff | Splinter Review
Make docshell privacy notifications work across multiple processes. (17.71 KB, patch)
2012-04-19 17:14 PDT, Josh Matthews [:jdm]
bzbarsky: review+
Details | Diff | Splinter Review

Description Josh Matthews [:jdm] 2012-02-21 10:52:21 PST
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.
Comment 1 Josh Matthews [:jdm] 2012-02-23 02:32:20 PST
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.
Comment 2 Josh Matthews [:jdm] 2012-04-15 22:43:13 PDT
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.
Comment 3 Josh Matthews [:jdm] 2012-04-17 00:51:16 PDT
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.
Comment 4 Josh Matthews [:jdm] 2012-04-17 00:55:39 PDT
The GetProcessType weirdness in nsDSURIContentListener is due to LockPref being unavailable in content processes and thus killing my IPC xpcshell test with an assertion.
Comment 5 Mozilla RelEng Bot 2012-04-17 01:02:27 PDT
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 Mozilla RelEng Bot 2012-04-17 01:02:31 PDT
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 Mozilla RelEng Bot 2012-04-17 04:16:11 PDT
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
Comment 8 Josh Matthews [:jdm] 2012-04-17 09:14:30 PDT
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.
Comment 9 Josh Matthews [:jdm] 2012-04-17 09:56:52 PDT
Created attachment 615756 [details] [diff] [review]
Make docshell privacy notifications work across multiple processes.
Comment 10 Mozilla RelEng Bot 2012-04-17 14:01:48 PDT
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 Mozilla RelEng Bot 2012-04-19 10:46:47 PDT
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 Mozilla RelEng Bot 2012-04-19 13:46:22 PDT
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 Mozilla RelEng Bot 2012-04-19 15:15:22 PDT
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
Comment 14 Josh Matthews [:jdm] 2012-04-19 17:14:57 PDT
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.
Comment 15 Boris Zbarsky [:bz] (TPAC) 2012-04-19 19:20:58 PDT
Comment on attachment 616817 [details] [diff] [review]
Make docshell privacy notifications work across multiple processes.

r=me
Comment 16 Josh Matthews [:jdm] 2012-04-19 20:11:06 PDT
Nevermind; something broke creating docshells in xpcshell tests since the last time I rebased.
Comment 17 Josh Matthews [:jdm] 2012-04-19 20:16:32 PDT
That was a lie; I just needed to rebuild.
Comment 18 Mozilla RelEng Bot 2012-04-19 21:30:49 PDT
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

Note You need to log in before you can comment on or make changes to this bug.