Closed Bug 1110495 Opened 5 years ago Closed 5 years ago

Wrap try/catch around e10s add-on shim initialization

Categories

(Toolkit :: General, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38
Iteration:
38.1 - 26 Jan
Tracking Status
e10s m5+ ---

People

(Reporter: mconley, Assigned: mconley)

Details

Attachments

(1 file)

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.
Flags: firefox-backlog+
Status: NEW → ASSIGNED
Attachment #8548487 - Flags: review?(wmccloskey)
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);
  }
}
Attachment #8548487 - Flags: review?(wmccloskey) → review+
(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!
Landed on fx-team with alterations:

remote:   https://hg.mozilla.org/integration/fx-team/rev/d0576cfa0b5c
https://hg.mozilla.org/mozilla-central/rev/d0576cfa0b5c
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Iteration: --- → 38.1 - 26 Jan
Flags: qe-verify?
You need to log in before you can comment on or make changes to this bug.