1347117 - IsContentParent() assertion failed when trying to focus on a nested remote frame.

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P2)

Description

[Evelyn Hung]

I'm trying out the patches on bug 558184 which is a nested content process setup, i.e.
Chrome -> content process -> jsplugin process
      \_________________________/

However, my tab crashed when mouse clicking on the pdf content. It is because in the content process the nsFoucusManager tries to call:

        if (TabParent* remote = TabParent::GetFrom(aContent)) {
          remote->Activate();

but in TabParent it assumes that it's under ContentParent's control, however, in our case it's under ContentBridgeParent's control. Therefore the assertion fails in:

        ContentParent* nsIContentParent::AsContentParent()
        {
          MOZ_ASSERT(IsContentParent());
          return static_cast<ContentParent*>(this);
        }

In bug 1334572 we moved Activate(and another two methods) from PBrowser to PContent, and introduced the logic above. I'm wondering how it supposes to work, and, should we add these methods to PContentBridge to resolve the nested case?

:billm, what do you think?

Comment 1

[Evelyn Hung]

Attachment: wip.patch

This wip added Activate/Deactivate/ParentActivated to PContentBridge to fix this problem. Tested and it works in our case.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8847079 [details] [diff] [review]
wip.patch

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

This looks fine to me. Sorry I broke it.

::: browser/extensions/mortar/host/pdf/bootstrap.js
@@ +50,5 @@
>        name: "PPAPI PDF plugin",
>        niceName: "PPAPI PDF plugin",
>        version: "1.0",
>        sandboxScript : `(${sandboxScript.toSource()})(this);`,
> +      ppapiProcessArgs: [ '-rpclib', rpclib, '-pluginlib', pluginlib ],

I don't think this should be in this patch.

Comment 3

[Evelyn Hung]

(In reply to Bill McCloskey (:billm) from comment #2)
> Comment on attachment 8847079 [details] [diff] [review]
> wip.patch
> 
> Review of attachment 8847079 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This looks fine to me. Sorry I broke it.
> 
> ::: browser/extensions/mortar/host/pdf/bootstrap.js
> @@ +50,5 @@
> >        name: "PPAPI PDF plugin",
> >        niceName: "PPAPI PDF plugin",
> >        version: "1.0",
> >        sandboxScript : `(${sandboxScript.toSource()})(this);`,
> > +      ppapiProcessArgs: [ '-rpclib', rpclib, '-pluginlib', pluginlib ],
> 
> I don't think this should be in this patch.

Oops! Right, it shouldn't. 

Thanks for the review. I will add commit messages and push try.

Comment 4

[Evelyn Hung]

try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e3f78741b283bea9359a358410a2121bc530bd4b

Comment 5

[Evelyn Hung]

Attachment: 0001-Bug-1347117.patch

carry over r+ from billm

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a600f0ea3fb
Add Activate/Deactivate/ParentActivated to PContentBridge; r=billm

Comment 7

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

https://hg.mozilla.org/mozilla-central/rev/1a600f0ea3fb