Closed Bug 1339014 Opened 3 years ago Closed 3 years ago

Add IProtocol::GetActorEventTarget() to Retrieve the EventTarget of the Actor if Set.

Categories

(Core :: IPC, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: bevis, Assigned: bevis)

References

Details

Attachments

(1 file)

I'd like to add this method for easier access to actor's event target, when it it's always set, for the following use cases instead of accessing from {CotentChild::GetSingleton(), BackgroundChild::GetForCurrentThread()}->GetActorEventTarget(*aActor):
1. Make EventTarget::Dispatch() easier in the callback of an actor child.
> BackgroundCursorChild::HandleResponse(const void_t& aResponse)
> {
>   ...
>   MOZ_ALWAYS_SUCCEEDS(this->GetActorEventTarget()->
>     Dispatch(deleteRunnable.forget(), NS_DISPATCH_NORMAL));
> }

2. Set event target to a actor managed by another IToplevelProtocol in the callback of an actor child.
> mozilla::ipc::IPCResult
> BackgroundFactoryRequestChild::RecvPermissionChallenge(
>                                             const PrincipalInfo& aPrincipalInfo)
> {
>   ...
>   RefPtr<TabChild> tabChild = mFactory->GetTabChild();
>   MOZ_ASSERT(tabChild);
> 
>   IPC::Principal ipcPrincipal(principal);
> 
>   auto* actor = new PermissionRequestChildProcessActor(this, mFactory);
> 
>   tabChild->SetEventTargetForActor(actor, this->GetActorEventTarget());
>   MOZ_ASSERT(actor->GetActorEventTarget());
>   tabChild->SendPIndexedDBPermissionRequestConstructor(actor, ipcPrincipal);
> }

3. Assertion to the actors whose event target was set or shall be inherit from its manager:
>   tabChild->SetEventTargetForActor(actor, this->GetActorEventTarget());
>   MOZ_ASSERT(actor->GetActorEventTarget());
>   tabChild->SendPIndexedDBPermissionRequestConstructor(actor, ipcPrincipal);
Or
> BackgroundDatabaseChild::RecvPBackgroundIDBVersionChangeTransactionConstructor(*aActor)
> {
>   ...
>   MOZ_ASSERT(aActor->GetActorEventTarget(),
>     "The event target shall be inherited from its manager actor.");
> }
Or
> nsresult
> IDBDatabase::Transaction(JSContext* aCx,
>                          const StringOrStringSequence& aStoreNames,
>                          IDBTransactionMode aMode,
>                          IDBTransaction** aTransaction)
> {
>   ...
>   BackgroundTransactionChild* actor =
>     new BackgroundTransactionChild(transaction);
> 
>   MOZ_ALWAYS_TRUE(
>     mBackgroundActor->SendPBackgroundIDBTransactionConstructor(actor,
>                                                                sortedStoreNames,
>                                                                mode));
>   MOZ_ASSERT(actor->GetActorEventTarget(),
>     "The event target shall be inherited from it manager actor.");
> }
Please see the reasons why this method is needed in comment 0.

Note: a raw nsIEventTarget* is returned instead of the already_addRefed<> one to make the code in call sites more compact. Raw pointer shall be fine because an assertion was added internally which assumes that the EventTarget that was set earlier could be retrieved during the actor's life cycle.

In addition, I assume that the EventTarget of IToplevelProtocol shall never be set according to current implementation to make the override of this method in IToplevelProtocol clear by returning nullptr. Maybe we could override IToplevelProtocol::{GetActorEventTarget(), SetEventTargetForActor(aActor, aEventTarget)} with MOZ_CRASH() instead if it makes more sense.
Attachment #8836637 - Flags: review?(wmccloskey)
Comment on attachment 8836637 [details] [diff] [review]
Patch: Add IProtocol::GetActorEventTarget() to Retrieve the EventTarget of the Actor if Set.

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

::: ipc/glue/ProtocolUtils.h
@@ +380,5 @@
>  
>      virtual void SetEventTargetForActorInternal(IProtocol* aActor, nsIEventTarget* aEventTarget);
>  
> +    virtual already_AddRefed<nsIEventTarget>
> +    GetActorEventTargetInternal(IProtocol* aActor) override;

The *override* here introduces more compiling errors to other member functions that *override* was not explicitly added (-Winconsistent-missing-override): 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ba8537a80e3672f26b10d9c9b8f4f2c236f240f0&selectedJob=76788640
I should remove it for now in next update.
Attachment #8836637 - Flags: review?(wmccloskey) → review+
Pushed by btseng@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/515db19cf529
Add IProtocol::GetActorEventTarget() to Retrieve the EventTarget of the Actor if Set. r=billm
https://hg.mozilla.org/mozilla-central/rev/515db19cf529
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.