977762 - Provide a mechanism to get the PContentParent associated with a PBackgroundParent

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v1

Various consumers will need a way to get the PContentParent from PBackgroundParent (for security checks, prompts, etc.). Patch attached to do that.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8383224 [details] [diff] [review]
Patch, v1

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

I complained to bent when he attached this patch that he was basically adding a giant foot-gun in the form of an already_AddRefed object that can only be destroyed from another thread. I argued in favor of an API where you pass a callback that is called with the already_AddRefed on the main thread. I challenged him to come up with a situation where the current API (which is more flexible) is demonstrably better than my proposal and he pointed out that Blobs will have use-cases where they need to use the content parent as an opaque pointer right then and there rather than do something useful on the main thread (other than release the pointer they've been given). This is still a little nerve-wracking since it means that we're going to be doing some extra work on the PBackground thread which is supposed to only dispatch things, but I trust him that this won't be a problem.

::: ipc/glue/BackgroundImpl.cpp
@@ +36,5 @@
>  
> +#ifdef RELEASE_BUILD
> +#define THREADSAFETY_ASSERT MOZ_ASSERT
> +#else
> +#define THREADSAFETY_ASSERT MOZ_RELEASE_ASSERT

Let the arms race begin!

::: ipc/glue/BackgroundParent.h
@@ +8,5 @@
>  #include "base/process.h"
>  #include "mozilla/Attributes.h"
>  #include "mozilla/ipc/Transport.h"
>  
> +template <class> class already_AddRefed;

This belongs to the other patch that I just reviewed, right?

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to Blake Kaplan (:mrbkap) from comment #1)
> This belongs to the other patch that I just reviewed, right?

No, I needed it because the new function returns already_AddRefed<ContentParent>

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cd8094ea37af

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cd8094ea37af