1337056 - Switch permission manager initialization to be async

Status: RESOLVED FIXED

(Core :: Permission Manager, defect)

Description

[(no longer active)]

Permission manager initialization can take a long time.

<https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2017-02-05&keys=PContent%253A%253AMsg_ReadPermissions&max_channel_version=nightly%252F54&measure=IPC_SYNC_LATENCY_MS&min_channel_version=null&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2017-02-02&table=0&trim=1&use_submission_date=0>

Start	End	IPC_SYNC_LATENCY_MS Count
0	1	0 (0%)
1	3	125.67k (41.87%)
3	8	44.81k (14.93%)
8	20	30.4k (10.13%)
20	50	27.56k (9.18%)
50	126	26.3k (8.76%)
126	317	24.91k (8.3%)
317	796	12.91k (4.3%)
796	2k	5.73k (1.91%)
2k	Infinity	1.83k (0.61%)

The other consumer of Msg_ReadPermissions is nsIPermissionManager::RefreshPermision() which is dead code being removed in bug 1337054.

Comment 1

[(no longer active)]

This is our 4th worst sync IPC per https://docs.google.com/spreadsheets/d/1x_BWVlnQPg0DHbsrvPFX7g89lnFGa3lAIHWD_pLa_dE/edit#gid=738048355.

Andrew can you please find an owner for this one?

Comment 2

[Andrew Overholt [:overholt]]

Michael said he'd take a look.

Comment 4

[(no longer active)]

I have a suggestion for fixing this: let's do something similar to the plan of bug 1331680, aka start the child process without the knowledge about any permissions, and only send down the permissions for the sites that you visit, in a way very similar to the proposal there (but for any LOAD_DOCUMENT_URI).  This has the additional privacy benefit that a rogue content process won't be able to query arbitrary permissions or modify them, only the ones for the websites loaded in that process.

For this to make sense we'd need to audit the TestPermission call sites.  I think in the content process we can return UNKNOWN_ACTION if the parent hasn't sent down the permission, and in case that does the wrong thing in a call site we find in the audit we can discuss further.

What do you think Michael?

Comment 5

[Nika Layzell [:nika] (ni? for response)]

(In reply to :Ehsan Akhgari from comment #4)
> I have a suggestion for fixing this: let's do something similar to the plan
> of bug 1331680, aka start the child process without the knowledge about any
> permissions, and only send down the permissions for the sites that you
> visit, in a way very similar to the proposal there (but for any
> LOAD_DOCUMENT_URI).  This has the additional privacy benefit that a rogue
> content process won't be able to query arbitrary permissions or modify them,
> only the ones for the websites loaded in that process.
> 
> For this to make sense we'd need to audit the TestPermission call sites.  I
> think in the content process we can return UNKNOWN_ACTION if the parent
> hasn't sent down the permission, and in case that does the wrong thing in a
> call site we find in the audit we can discuss further.
> 
> What do you think Michael?

That sounds like a neat idea. Unfortunately, we currently provide an API for enumerating all permissions (nsIPermissionManager.enumerator), and that would produce slightly odd different results under this model. I'm not completely sure if we care.

I quickly threw together a patch today which simply pushes all of the permissions to the child process as efficiently as possible and without blocking any event loops - We can use something like it as a fallback if this patch doesn't work out. It avoids a bunch of computation work and thread blocking compared to the current implementation, so it should be an improvement whether or not this works out. I'm worried about its worst case situation, however, as I have seen permissions databases which have tens of thousands of entries due to some add-on or another.

Would you like me to take over bug 1331680, as it seems like a subset of the implementation work required for your ideas here (especially because it only has one consumer)?

Comment 6

[(no longer active)]

(In reply to Michael Layzell [:mystor] from comment #5)
> (In reply to :Ehsan Akhgari from comment #4)
> > I have a suggestion for fixing this: let's do something similar to the plan
> > of bug 1331680, aka start the child process without the knowledge about any
> > permissions, and only send down the permissions for the sites that you
> > visit, in a way very similar to the proposal there (but for any
> > LOAD_DOCUMENT_URI).  This has the additional privacy benefit that a rogue
> > content process won't be able to query arbitrary permissions or modify them,
> > only the ones for the websites loaded in that process.
> > 
> > For this to make sense we'd need to audit the TestPermission call sites.  I
> > think in the content process we can return UNKNOWN_ACTION if the parent
> > hasn't sent down the permission, and in case that does the wrong thing in a
> > call site we find in the audit we can discuss further.
> > 
> > What do you think Michael?
> 
> That sounds like a neat idea. Unfortunately, we currently provide an API for
> enumerating all permissions (nsIPermissionManager.enumerator), and that
> would produce slightly odd different results under this model. I'm not
> completely sure if we care.

Yeah I think that's OK.  But this is another thing to watch for in the audit.  I think it's OK if some call sites need to be modified to do the right thing post this change.

> I quickly threw together a patch today which simply pushes all of the
> permissions to the child process as efficiently as possible and without
> blocking any event loops - We can use something like it as a fallback if
> this patch doesn't work out. It avoids a bunch of computation work and
> thread blocking compared to the current implementation, so it should be an
> improvement whether or not this works out. I'm worried about its worst case
> situation, however, as I have seen permissions databases which have tens of
> thousands of entries due to some add-on or another.

Yeah the permission manager DB can be quite large.  However in the worst case we should be able to send the data from parent to child asynchronously after the child starts, but we have to be careful to deal with the possibility that the permission manager may need the data before it has arrived.  With my suggestion this problem sort of fixes itself.

> Would you like me to take over bug 1331680, as it seems like a subset of the
> implementation work required for your ideas here (especially because it only
> has one consumer)?

No, I think Amy is going to work on that bug.  I don't think we can really share code since the cookie stuff runs off of the PCookieService protocol under PNecko but here we probably want to use PContent directly, etc.  And the logic doesn't match between these two bugs in any way other than the abstract idea.  :-)  So I think these can be done independently in parallel.

Comment 7

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 1: Add a mechanism for grouping permissions into groups to be sent over IPC

This is a first pass implementation of asynchronous permission communication to the content process. It handles the primary protocols which permissions can be found on (http[s]:// and ftp://).

I plan to add support for file:// in a follow up (which is more complex due to the protocol not performing IPC to load the content).

MozReview-Commit-ID: IQiSsVGaAOQ

Comment 8

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 2: Replace the synchronous ReadPermissions API with async APIs

These APIs are intended to use the mechanism defined in Part 1.

Part 3 implements the usage of these APIs to synchronize permissions.

MozReview-Commit-ID: HNKyDPtoaHl

Comment 9

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 3: Send down http[s] and ftp permissions as they are needed. Send down other permissions at startup

MozReview-Commit-ID: CUKPvFp6zpF

Comment 10

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 4: Assert that the ipcKey for a principal is avaliable when creating a PermissionKey in the child process

MozReview-Commit-ID: G9TynCKgCVF

Comment 11

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 5: Add support for finer-grained permission update messages to ContentParent

MozReview-Commit-ID: 6y6eBLWPTue

Comment 12

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 6: Disable nsPermissionManager::GetEnumerator in the content process

This property allows enumerating over all permissions. As the content
process no longer has access to all permissions, this method cannot be
correctly implemented in the content process anymore.

Because of that, we now error with NS_ERROR_NOT_AVALIABLE when it is
accessed in the content process.

MozReview-Commit-ID: BLNeYYcZhIi

Comment 13

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 7: Refactor nsObjectLoadingContent::GetTypeOfContent logic out into nsContentUtils

MozReview-Commit-ID: IJQNhQZzx3y

Comment 14

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 8: Add nsIRequest::LOAD_HTML_OBJECT_DATA flag to identify requests loaded by nsObjectLoadingContent

MozReview-Commit-ID: 3hgLRCeuiyD

Comment 15

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 9: Check LOAD_HTML_OBJECT_DATA in ContentParent::TransmitPermsFor

MozReview-Commit-ID: 5MBAUe5zbvU

Comment 16

[Nika Layzell [:nika] (ni? for response)]

Doing a try push to make sure the assertions aren't firing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bdf3a75d725e996c81072ec9f2389d7c54532c45

Comment 17

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8844086 [details] [diff] [review]
Part 1: Add a mechanism for grouping permissions into groups to be sent over IPC

Moving a bunch of reviews from ehsan->baku as we've talked about this plan too much together and it would be good to get another set of eyes on the idea.

Comment 18

[Lina Butler [:lina]]

Comment on attachment 8844091 [details] [diff] [review]
Part 6: Disable nsPermissionManager::GetEnumerator in the content process

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

Looking closer, I think you uncovered another bug in `NotificationTelemetryService::Init`. It should only enumerate permissions and add a permission observer in the parent at startup. IIUC, though, `Init` will run when we get the service for the first time in the content process, meaning it'll add another observer and double-count permission changes.

I know this was already broken, but would you mind changing `Init` to bail early if it's in the content process, and asserting that `AddPermissionChangeObserver` and `RecordPermissions` are only ever called in the parent?

Thanks for catching this!

Comment 19

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 6: Disable nsPermissionManager::GetEnumerator in the content process

This property allows enumerating over all permissions. As the content
process no longer has access to all permissions, this method cannot be
correctly implemented in the content process anymore.

Because of that, we now error with NS_ERROR_NOT_AVALIABLE when it is
accessed in the content process.

MozReview-Commit-ID: BLNeYYcZhIi

Comment 20

[Lina Butler [:lina]]

Comment on attachment 8844111 [details] [diff] [review]
Part 6: Disable nsPermissionManager::GetEnumerator in the content process

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

Thanks!

Comment 21

[Andrea Marchesini [:baku]]

Comment on attachment 8844088 [details] [diff] [review]
Part 3: Send down http[s] and ftp permissions as they are needed. Send down other permissions at startup

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

::: dom/ipc/ContentParent.cpp
@@ +4982,5 @@
>    ProcessHangMonitor::ForcePaint(mHangMonitorActor, aTabParent, aLayerObserverEpoch);
>  }
>  
> +nsresult
> +ContentParent::TransmitPermsFor(nsIChannel* aChannel)

Permissions instead Perms? In all our code base we have 'Permissions'.

@@ +4983,5 @@
>  }
>  
> +nsresult
> +ContentParent::TransmitPermsFor(nsIChannel* aChannel)
> +{

MOZ_ASSERT(aChannel);

::: netwerk/protocol/ftp/FTPChannelParent.cpp
@@ +461,5 @@
>  
> +  // Send down any permissions which are relevant to this URL if we are
> +  // performing a document load.
> +  PContentParent* pcp = Manager()->Manager();
> +  static_cast<ContentParent*>(pcp)->TransmitPermsFor(chan);

this can fail.

::: netwerk/protocol/http/HttpChannelParent.cpp
@@ +1120,5 @@
>  
> +  // Send down any permissions which are relevant to this URL if we are
> +  // performing a document load.
> +  PContentParent* pcp = Manager()->Manager();
> +  static_cast<ContentParent*>(pcp)->TransmitPermsFor(chan);

this can fail.

::: netwerk/protocol/wyciwyg/WyciwygChannelParent.cpp
@@ +324,5 @@
> +  // Send down any permissions which are relevant to this URL if we are
> +  // performing a document load.
> +  PContentParent* pcp = Manager()->Manager();
> +  static_cast<ContentParent*>(pcp)->TransmitPermsFor(chan);
> +

this can fail.

Comment 22

[Andrea Marchesini [:baku]]

Comment on attachment 8844087 [details] [diff] [review]
Part 2: Replace the synchronous ReadPermissions API with async APIs

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

I don't like ipcKey. Can you describe better what it is in the nsIPermissionManager.idl and change its name?

::: dom/ipc/PContent.ipdl
@@ +696,5 @@
>      async Deactivate(PBrowser aTab);
>  
>      async ParentActivated(PBrowser aTab, bool aActivated);
>  
> +    async SetPermissionsWithKey(nsCString aIpcKey, Permission[] aPermissions);

aPermissionKey ?

::: extensions/cookie/nsPermissionManager.cpp
@@ +2892,5 @@
>    return NS_OK;
>  }
>  
> +NS_IMETHODIMP
> +nsPermissionManager::GetPermissionsWithKey(const nsACString& aIpcKey,

aPermissionKey ? aIPCKey is too generic.

@@ +2916,5 @@
> +    }
> +
> +    // Get the IPC Key and make sure that it matches the aIpcKey passed in.
> +    nsAutoCString ipcKey;
> +    GetKeyForPrincipal(principal, ipcKey);

here you should deal with an error.

Comment 23

[Andrea Marchesini [:baku]]

Comment on attachment 8844086 [details] [diff] [review]
Part 1: Add a mechanism for grouping permissions into groups to be sent over IPC

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

Propagate the errors.

::: extensions/cookie/nsPermissionManager.cpp
@@ +2936,5 @@
> +
> +// XXX: Support file URIs here as well!
> +/* static */ void
> +nsPermissionManager::GetKeyForPrincipal(nsIPrincipal* aPrincipal, nsACString& aKey)
> +{

MOZ_ASSERT(aPrincipal);

@@ +2942,5 @@
> +
> +  nsCOMPtr<nsIURI> uri;
> +  nsresult rv = aPrincipal->GetURI(getter_AddRefs(uri));
> +  if (NS_WARN_IF(NS_FAILED(rv))) {
> +    return;

why no error propagation?

Comment 24

[Andrea Marchesini [:baku]]

Comment on attachment 8844090 [details] [diff] [review]
Part 5: Add support for finer-grained permission update messages to ContentParent

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

::: extensions/cookie/nsPermissionManager.cpp
@@ +1567,5 @@
>      IPC::Permission permission(origin, aType, aPermission,
>                                 aExpireType, aExpireTime);
>  
> +    nsAutoCString ipcKey;
> +    GetKeyForPrincipal(aPrincipal, ipcKey);

This could fail... see comments in previous reviews.

Comment 25

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 1: Add a mechanism for grouping permissions into groups to be sent over IPC

I have made the name changes (No longer called IPC key) that you requested. I also added comments explaining the reason why GetKeyForPrincipal is infallible. 

MozReview-Commit-ID: IQiSsVGaAOQ

Comment 26

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 2: Replace the synchronous ReadPermissions API with async APIs

These APIs are intended to use the mechanism defined in Part 1.

Part 3 implements the usage of these APIs to synchronize permissions.

MozReview-Commit-ID: HNKyDPtoaHl

Comment 27

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 3: Send down http[s] and ftp permissions as they are needed. Send down other permissions at startup

MozReview-Commit-ID: CUKPvFp6zpF

Comment 28

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 4: Assert that the ipcKey for a principal is avaliable when creating a PermissionKey in the child process

MozReview-Commit-ID: G9TynCKgCVF

Comment 29

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 5: Add support for finer-grained permission update messages to ContentParent

MozReview-Commit-ID: 6y6eBLWPTue

Comment 30

[Nika Layzell [:nika] (ni? for response)]

You asked me to NI? you once the new patches went up :)

Comment 31

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 10: Remove ReadPermissions from the sync-messages list

I was just getting ready to finally actually land this and realized I hadn't removed the call from the sync IPC list.

MozReview-Commit-ID: 8Ki4KEHKnQm

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7568383eb18d
Part 1: Add a mechanism for grouping permissions into groups to be sent over IPC, r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/2d2d603dbdaf
Part 2: Replace the synchronous ReadPermissions API with async APIs, r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9674c8a9d4f
Part 3: Send down http[s] and ftp permissions as they are needed. Send down other permissions at startup, r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a6e7570df3e
Part 4: Assert that the ipcKey for a principal is avaliable when creating a PermissionKey in the child process, r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e4ee0af3ff0
Part 5: Add support for finer-grained permission update messages to ContentParent, r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/33447647d4e8
Part 6: Disable nsPermissionManager::GetEnumerator in the content process, r=kitcambridge
https://hg.mozilla.org/integration/mozilla-inbound/rev/cca9db8d2256
Part 7: Refactor nsObjectLoadingContent::GetTypeOfContent logic out into nsContentUtils, r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/40b8605b5e48
Part 8: Add nsIRequest::LOAD_HTML_OBJECT_DATA flag to identify requests loaded by nsObjectLoadingContent, r=mcmanus
https://hg.mozilla.org/integration/mozilla-inbound/rev/31a996094ef4
Part 9: Check LOAD_HTML_OBJECT_DATA in ContentParent::TransmitPermsFor, r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/0690f70aa69a
Part 10: Remove ReadPermissions from the sync-messages list, r=billm

Comment 33

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out bug 1337056 and bug 1345573 for crashing xpcshell test test_bug930456_child.js on debug:

bug 1337056:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba0c83461506c421e5c7473cce9a53d1bfee06d8
https://hg.mozilla.org/integration/mozilla-inbound/rev/ddcfb1e70d7230c491dc0a2cb756320de28eea17
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd3ecc8e83179b40e50e39645b9efa7e53fda7d5
https://hg.mozilla.org/integration/mozilla-inbound/rev/36c22158aa94be8bbe82e970462a36b157f775e1
https://hg.mozilla.org/integration/mozilla-inbound/rev/b4b5387adabe9cabf299ca25ea22a42df00511ca
https://hg.mozilla.org/integration/mozilla-inbound/rev/95a8674a048d7ce09de02b9340e075b321bcb68a
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc53ad27cd41785974bc83cf2d015a7cf1bee33a
https://hg.mozilla.org/integration/mozilla-inbound/rev/a7b2c6dacc63afc23dcaffe377a9e4b1df2d1b30
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b93025f476945108ac6988ad00a61f3c54f331b
https://hg.mozilla.org/integration/mozilla-inbound/rev/a5fb7e5eb46b2083a6c424307c961bd255458f83

bug 1345573:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8767bffeadca0b8d907eb251149965745ecd8b85
https://hg.mozilla.org/integration/mozilla-inbound/rev/30bb3bc160d3e6022892c717c00d114695546a07

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=f5f05a774023c7e9cc8eedbfbc91ba117f49274a&filter-resultStatus=runnable&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=83785901&repo=mozilla-inbound

12:53:31  WARNING -  PROCESS-CRASH | toolkit/components/search/tests/xpcshell/test_bug930456_child.js | application crashed [@ mozalloc_abort(char const*)]
12:53:31     INFO -  Crash dump filename: /var/folders/64/m7jhlc3x3b32lz348yvd80pc00000w/T/xpc-other-OWG_6b/0B714EBA-12B1-4D08-B04E-DE790AF0F45F.dmp
12:53:31     INFO -  Operating system: Mac OS X
12:53:31     INFO -                    10.10.5 14F27
12:53:31     INFO -  CPU: amd64
12:53:31     INFO -       family 6 model 69 stepping 1
12:53:31     INFO -       4 CPUs
12:53:31     INFO -  GPU: UNKNOWN
12:53:31     INFO -  Crash reason:  EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
12:53:31     INFO -  Crash address: 0x0
12:53:31     INFO -  Process uptime: 6 seconds
12:53:31     INFO -  Thread 0 (crashed)
12:53:31     INFO -   0  libmozglue.dylib!mozalloc_abort(char const*) [mozalloc_abort.cpp:f5f05a774023 : 33 + 0x0]
12:53:31     INFO -      rax = 0x0000000000000000   rdx = 0x00007fff7c5691f8
12:53:31     INFO -      rcx = 0x0000000000000000   rbx = 0x00007fff7c569c50
12:53:31     INFO -      rsi = 0x00002b0000002b00   rdi = 0x00002a0000002b03
12:53:31     INFO -      rbp = 0x00007fff534e4c20   rsp = 0x00007fff534e4c10
12:53:31     INFO -       r8 = 0x00007fff534e4bc0    r9 = 0x00007fff7c4f4300
12:53:31     INFO -      r10 = 0x0000000000000000   r11 = 0x0000000000000246
12:53:31     INFO -      r12 = 0x0000000000000001   r13 = 0x0000000111872568
12:53:31     INFO -      r14 = 0x00007fff534e4c90   r15 = 0x00007fff534e6e8b
12:53:31     INFO -      rip = 0x0000000113032439
12:53:31     INFO -      Found by: given as instruction pointer in context
12:53:31     INFO -   1  XUL!Abort(char const*) [nsDebugImpl.cpp:f5f05a774023 : 441 + 0x5]
12:53:31     INFO -      rbx = 0x00007fff7c569c50   rbp = 0x00007fff534e4c30
12:53:31     INFO -      rsp = 0x00007fff534e4c30   r12 = 0x0000000000000001
12:53:31     INFO -      r13 = 0x0000000111872568   r14 = 0x00007fff534e4c90
12:53:31     INFO -      r15 = 0x00007fff534e6e8b   rip = 0x000000010ca3d7a9
12:53:31     INFO -      Found by: call frame info
12:53:31     INFO -   2  XUL!NS_DebugBreak [nsDebugImpl.cpp:f5f05a774023 : 428 + 0x8]
12:53:31     INFO -      rbx = 0x00007fff7c569c50   rbp = 0x00007fff534e50c0
12:53:31     INFO -      rsp = 0x00007fff534e4c40   r12 = 0x0000000000000001
12:53:31     INFO -      r13 = 0x0000000111872568   r14 = 0x00007fff534e4c90
12:53:31     INFO -      r15 = 0x00007fff534e6e8b   rip = 0x000000010ca3d590
12:53:31     INFO -      Found by: call frame info
12:53:31     INFO -   3  XUL!nsPermissionManager::PermissionKey::CreateFromPrincipal(nsIPrincipal*, nsresult&) [nsPermissionManager.cpp:f5f05a774023 : 661 + 0x1e]
12:53:31     INFO -      rbx = 0x00007fff534e50d8   rbp = 0x00007fff534e51f0
12:53:31     INFO -      rsp = 0x00007fff534e50d0   r12 = 0x00007fff534e5168
12:53:31     INFO -      r13 = 0x0000000000000000   r14 = 0x0000000119482920
12:53:31     INFO -      r15 = 0x00007fff534e5128   rip = 0x000000010d5ecc76
12:53:31     INFO -      Found by: call frame info
12:53:31     INFO -   4  XUL!nsPermissionManager::AddInternal(nsIPrincipal*, nsCString const&, unsigned int, long long, unsigned int, long long, long long, nsPermissionManager::NotifyOperationType, nsPermissionManager::DBOperationType, bool) [nsPermissionManager.cpp:f5f05a774023 : 1638 + 0xc]
12:53:31     INFO -      rbx = 0x0000000119482920   rbp = 0x00007fff534e5390
12:53:31     INFO -      rsp = 0x00007fff534e5200   r12 = 0x0000000113362980
12:53:31     INFO -      r13 = 0x0000000000000000   r14 = 0x00007fff534e5300
12:53:31     INFO -      r15 = 0x0000000000000000   rip = 0x000000010d5f4aff
12:53:31     INFO -      Found by: call frame info
12:53:31     INFO -   5  XUL!mozilla::dom::ContentChild::RecvAddPermission(IPC::Permission const&) [ContentChild.cpp:f5f05a774023 : 2234 + 0x25]
12:53:31     INFO -      rbx = 0x0000000119482920   rbp = 0x00007fff534e54b0
12:53:31     INFO -      rsp = 0x00007fff534e53a0   r12 = 0x00007fff534e55a0
12:53:31     INFO -      r13 = 0x00007fff534e5420   r14 = 0x0000000113362980
12:53:31     INFO -      r15 = 0x00007fff534e53f8   rip = 0x000000010f1d396c
12:53:31     INFO -      Found by: call frame info
12:53:31     INFO -   6  XUL!mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) [PContentChild.cpp:f5f05a774023 : 6832 + 0x13]
12:53:31     INFO -      rbx = 0x00007fff534e56b0   rbp = 0x00007fff534e5700
12:53:31     INFO -      rsp = 0x00007fff534e54c0   r12 = 0x0000000113393bc8
12:53:31     INFO -      r13 = 0x00007fff534e55b0   r14 = 0x00000001133ac020
12:53:31     INFO -      r15 = 0x0000000113393bd0   rip = 0x000000010d40eba9
12:53:31     INFO -      Found by: call frame info
12:53:31     INFO -   7  XUL!mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) [MessageChannel.cpp:f5f05a774023 : 1872 + 0x9]
12:53:31     INFO -      rbx = 0x0000000000000001   rbp = 0x00007fff534e5760
12:53:31     INFO -      rsp = 0x00007fff534e5710   r12 = 0x0000000000000000
12:53:31     INFO -      r13 = 0x00000001133ac118   r14 = 0x0000000113393bc8
12:53:31     INFO -      r15 = 0x00000000ffffff00   rip = 0x000000010d0893cb
12:53:31     INFO -      Found by: call frame info
12:53:31     INFO -   8  XUL!mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) [MessageChannel.cpp:f5f05a774023 : 1807 + 0xb]
12:53:31     INFO -      rbx = 0x0000000113389330   rbp = 0x00007fff534e5820
12:53:31     INFO -      rsp = 0x00007fff534e5770   r12 = 0x0000000113393bc8
12:53:31     INFO -      r13 = 0x0000000113306e70   r14 = 0x00000001133ac118
12:53:31     INFO -      r15 = 0x00000000ffffffff   rip = 0x000000010d087197
12:53:31     INFO -      Found by: call frame info
12:53:31     INFO -   9  XUL!mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) [MessageChannel.cpp:f5f05a774023 : 1680 + 0xb]
12:53:31     INFO -      rbx = 0x0000000113393b00   rbp = 0x00007fff534e5880
12:53:31     INFO -      rsp = 0x00007fff534e5830   r12 = 0x0000000113393b70
12:53:31     INFO -      r13 = 0x0000000113306e70   r14 = 0x00000001133ac118
12:53:31     INFO -      r15 = 0x0000000113393bc8   rip = 0x000000010d0883a2
12:53:31     INFO -      Found by: call frame info
12:53:31     INFO -  10  XUL!mozilla::ipc::MessageChannel::MessageTask::Run() [MessageChannel.cpp:f5f05a774023 : 1713 + 0x8]
12:53:31     INFO -      rbx = 0x0000000113393b70   rbp = 0x00007fff534e58a0
12:53:31     INFO -      rsp = 0x00007fff534e5890   r12 = 0x00007fff534e5918
12:53:31     INFO -      r13 = 0x0000000113306e70   r14 = 0x0000000113389330
12:53:31     INFO -      r15 = 0x00000001133c8040   rip = 0x000000010d088b48
12:53:31     INFO -      Found by: call frame info
12:53:31     INFO -  11  XUL!nsThread::ProcessNextEvent(bool, bool*) [nsThread.cpp:f5f05a774023 : 1269 + 0x6]
12:53:31     INFO -      rbx = 0x00007fff534e5900   rbp = 0x00007fff534e59a0
12:53:31     INFO -      rsp = 0x00007fff534e58b0   r12 = 0x00007fff534e5918
12:53:31     INFO -      r13 = 0x0000000113306e70   r14 = 0x00000001133c8090
12:53:31     INFO -      r15 = 0x00000001133c8040   rip = 0x000000010caf939c
12:53:31     INFO -      Found by: call frame info
12:53:31     INFO -  12  XUL!NS_ProcessNextEvent(nsIThread*, bool) [nsThreadUtils.cpp:f5f05a774023 : 389 + 0xd]
12:53:31     INFO -      rbx = 0x0000000000000000   rbp = 0x00007fff534e59d0
12:53:31     INFO -      rsp = 0x00007fff534e59b0   r12 = 0x0000000000000001
12:53:31     INFO -      r13 = 0x0000000113306e70   r14 = 0x00007fff534e5bd8
12:53:31     INFO -      r15 = 0x0000000000000001   rip = 0x000000010caf798e
12:53:31     INFO -      Found by: call frame info
12:53:31     INFO -  13  XUL!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) [MessagePump.cpp:f5f05a774023 : 96 + 0xb]
12:53:31     INFO -      rbx = 0x0000000113306e90   rbp = 0x00007fff534e5a40
12:53:31     INFO -      rsp = 0x00007fff534e59e0   r12 = 0x0000000000000001
12:53:31     INFO -      r13 = 0x0000000113306e70   r14 = 0x00007fff534e5bd8
12:53:31     INFO -      r15 = 0x0000000000000001   rip = 0x000000010d08c833
12:53:31     INFO -      Found by: call frame info
12:53:31     INFO -  14  XUL!MessageLoop::Run() [message_loop.cc:f5f05a774023 : 231 + 0x5]
12:53:31     INFO -      rbx = 0x000000011961a640   rbp = 0x00007fff534e5a70
12:53:31     INFO -      rsp = 0x00007fff534e5a50   r12 = 0x0000000000000003
12:53:31     INFO -      r13 = 0x00007fff534e5e40   r14 = 0x00000000534e5b01
12:53:31     INFO -      r15 = 0x00007fff534e5bd8   rip = 0x000000010d040efc
12:53:31     INFO -      Found by: call frame info
12:53:31     INFO -  15  XUL!XRE_RunAppShell() [nsEmbedFunctions.cpp:f5f05a774023 : 866 + 0x8]
12:53:31     INFO -      rbx = 0x000000011961a640   rbp = 0x00007fff534e5ac0
12:53:31     INFO -      rsp = 0x00007fff534e5a80   r12 = 0x0000000000000003
12:53:31     INFO -      r13 = 0x00007fff534e5e40   r14 = 0x00000000534e5b01
12:53:31     INFO -      r15 = 0x00007fff534e5bd8   rip = 0x0000000110a63683
12:53:31     INFO -      Found by: call frame info
12:53:31     INFO -  16  XUL!mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) [MessagePump.cpp:f5f05a774023 : 269 + 0x5]
12:53:31     INFO -      rbx = 0x00007fff534e5bd8   rbp = 0x00007fff534e5af0
12:53:31     INFO -      rsp = 0x00007fff534e5ad0   r12 = 0x0000000000000003
12:53:31     INFO -      r13 = 0x00007fff534e5e40   r14 = 0x0000000113306e70
12:53:31     INFO -      r15 = 0x0000000080004005   rip = 0x000000010d08d282
12:53:31     INFO -      Found by: call frame info
12:53:31     INFO -  17  XUL!MessageLoop::Run() [message_loop.cc:f5f05a774023 : 231 + 0x5]
12:53:31     INFO -      rbx = 0x0000000000000001   rbp = 0x00007fff534e5b20
12:53:31     INFO -      rsp = 0x00007fff534e5b00   r12 = 0x0000000000000003
12:53:31     INFO -      r13 = 0x00007fff534e5e40   r14 = 0x0000000000000012
12:53:31     INFO -      r15 = 0x0000000080004005   rip = 0x000000010d040efc
12:53:31     INFO -      Found by: call frame info
12:53:31     INFO -  18  XUL!XRE_InitChildProcess(int, char**, XREChildData const*) [nsEmbedFunctions.cpp:f5f05a774023 : 695 + 0x5]
12:53:31     INFO -      rbx = 0x0000000000000001   rbp = 0x00007fff534e5de0
12:53:31     INFO -      rsp = 0x00007fff534e5b30   r12 = 0x0000000000000003
12:53:31     INFO -      r13 = 0x00007fff534e5e40   r14 = 0x0000000000000012
12:53:31     INFO -      r15 = 0x0000000080004005   rip = 0x0000000110a63297
12:53:31     INFO -      Found by: call frame info
12:53:31     INFO -  19  plugin-container!main [plugin-container.cpp:f5f05a774023 : 64 + 0x13]
12:53:31     INFO -      rbx = 0x0000000113334188   rbp = 0x00007fff534e5e20
12:53:31     INFO -      rsp = 0x00007fff534e5df0   r12 = 0x0000000000000000
12:53:31     INFO -      r13 = 0x0000000000000000   r14 = 0x00007fff534e5e40
12:53:31     INFO -      r15 = 0x0000000000000015   rip = 0x000000010c719ee9
12:53:31     INFO -      Found by: call frame info
12:53:31     INFO -  20  plugin-container!start + 0x34
12:53:31     INFO -      rbx = 0x0000000000000000   rbp = 0x00007fff534e5e30
12:53:31     INFO -      rsp = 0x00007fff534e5e30   r12 = 0x0000000000000000
12:53:31     INFO -      r13 = 0x0000000000000000   r14 = 0x0000000000000000
12:53:31     INFO -      r15 = 0x0000000000000000   rip = 0x000000010c719e84
12:53:31     INFO -      Found by: call frame info

Comment 34

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 12: Send down permissions when calling createAboutBlankContentViewer

Ugh. Every time I do another try push thinking it's the last one I find new problems which were hidden before.

I hope this is the last part :).

MozReview-Commit-ID: LigZnHM34CC

Comment 35

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 11: Change the permission key assertion to a fatal assert on debug builds

MozReview-Commit-ID: HTxvlomRKWy

Comment 36

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff6d2ebc113f
Part 1: Add a mechanism for grouping permissions into groups to be sent over IPC, r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/215ca1b54ff1
Part 2: Replace the synchronous ReadPermissions API with async APIs, r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2b8ead5376b
Part 3: Send down http[s] and ftp permissions as they are needed. Send down other permissions at startup, r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc5c70cb1511
Part 4: Assert that the ipcKey for a principal is avaliable when creating a PermissionKey in the child process, r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/a21d709fd0b0
Part 5: Add support for finer-grained permission update messages to ContentParent, r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/d01b69497592
Part 6: Disable nsPermissionManager::GetEnumerator in the content process, r=kitcambridge
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc03c82cb565
Part 7: Refactor nsObjectLoadingContent::GetTypeOfContent logic out into nsContentUtils, r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c41abe5360d
Part 8: Add nsIRequest::LOAD_HTML_OBJECT_DATA flag to identify requests loaded by nsObjectLoadingContent, r=mcmanus
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ea56abef5ea
Part 9: Check LOAD_HTML_OBJECT_DATA in ContentParent::TransmitPermsFor, r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/55fe81687988
Part 10: Remove ReadPermissions from the sync-messages list, r=billm
https://hg.mozilla.org/integration/mozilla-inbound/rev/2686c6ee997d
Part 11: Change the permission key assertion to a fatal assert on debug builds, r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/f86ef08fdae0
Part 12: Send down permissions when calling createAboutBlankContentViewer, r=baku

Comment 37

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ff6d2ebc113f
https://hg.mozilla.org/mozilla-central/rev/215ca1b54ff1
https://hg.mozilla.org/mozilla-central/rev/c2b8ead5376b
https://hg.mozilla.org/mozilla-central/rev/dc5c70cb1511
https://hg.mozilla.org/mozilla-central/rev/a21d709fd0b0
https://hg.mozilla.org/mozilla-central/rev/d01b69497592
https://hg.mozilla.org/mozilla-central/rev/cc03c82cb565
https://hg.mozilla.org/mozilla-central/rev/8c41abe5360d
https://hg.mozilla.org/mozilla-central/rev/8ea56abef5ea
https://hg.mozilla.org/mozilla-central/rev/55fe81687988
https://hg.mozilla.org/mozilla-central/rev/2686c6ee997d
https://hg.mozilla.org/mozilla-central/rev/f86ef08fdae0

Comment 38

[Toshihiro Yamada]

Is this crash(bp-d67b8fd2-3107-4e34-875a-eeaa72170322) related to these changes?

Comment 39

[(no longer active)]

(In reply to Toshihiro Yamada from comment #38)
> Is this crash(bp-d67b8fd2-3107-4e34-875a-eeaa72170322) related to these
> changes?

Yeah, filed bug 1349634.