Closed Bug 977762 Opened 6 years ago Closed 6 years ago

Provide a mechanism to get the PContentParent associated with a PBackgroundParent

Categories

(Core :: IPC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: bent.mozilla, Assigned: bent.mozilla)

Details

Attachments

(1 file)

Attached patch Patch, v1Splinter Review
Various consumers will need a way to get the PContentParent from PBackgroundParent (for security checks, prompts, etc.). Patch attached to do that.
Attachment #8383224 - Flags: review?(mrbkap)
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?
Attachment #8383224 - Flags: review?(mrbkap) → review+
(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>
https://hg.mozilla.org/mozilla-central/rev/cd8094ea37af
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.