1110495 - Wrap try/catch around e10s add-on shim initialization

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

We've seen from bug 1102410 that it's possible for the initialization of the add-on shims to cause pretty major problems for remote browsers, since throwing an exception inside browser-child.js can cause pretty important stuff to not execute.

Examples include: autocomplete for form fields, and session history (youch!)

We should wrap the initialization of the shims in a try/catch, report anything that goes wrong while initializing, but let the remainder of browser-child.js execute.

Marking this M5 because this is simple to do, and probably just good defensive coding. Not going with M4 because I'm a coward.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Attachment: Wrap try/catch around e10s add-on shim initialization. r=?

Comment 2

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

Comment on attachment 8548487 [details] [diff] [review]
Wrap try/catch around e10s add-on shim initialization. r=?

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

::: toolkit/components/addoncompat/RemoteAddonsChild.jsm
@@ +526,5 @@
> +    try {
> +      ObserverChild.init();
> +    } catch(e) {
> +      Cu.reportError(e);
> +    }

How about:
let shims = [ObserverChild, ...];
for (let shim of shims) {
  try {
    shim.init();
  } catch (e) {
    Cu.reportError(e);
  }
}

Comment 3

[Mike Conley (:mconley) (:⚙️)]

(In reply to Bill McCloskey (:billm) from comment #2)
> Comment on attachment 8548487 [details] [diff] [review]
> Wrap try/catch around e10s add-on shim initialization. r=?
> 
> Review of attachment 8548487 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/components/addoncompat/RemoteAddonsChild.jsm
> @@ +526,5 @@
> > +    try {
> > +      ObserverChild.init();
> > +    } catch(e) {
> > +      Cu.reportError(e);
> > +    }
> 
> How about:
> let shims = [ObserverChild, ...];
> for (let shim of shims) {
>   try {
>     shim.init();
>   } catch (e) {
>     Cu.reportError(e);
>   }
> }

Yeah, that's way better. Thanks!

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Landed on fx-team with alterations:

remote:   https://hg.mozilla.org/integration/fx-team/rev/d0576cfa0b5c

Comment 5

[Carsten Book [:Tomcat]]

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