523994 - Clean up control flow in XPCConvert::NativeInterface2JSObject

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Blake Kaplan (:mrbkap) (inactive)]

As I sat down to patch XPCConvert::NativeInterface2JSObject today, I realized that I had no clue how the control flow in it works. This is because there is an absolute mess of early returns coupled with if statements that sometimes fall through, etc, etc.

I'm trying to clean it up, using early returns to exclude cases when we don't need to care about them anymore.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: wip

This patch is broken somewhere (the browser actually sort of starts, but can't bring up a window). Unfortunately, my laptop is running out of batteries and I'm on an airplane, so I'll have to leave this for now. I'll pick it up again tomorrow.

There are some XXX comments I might want to address before landing this, in addition to fixing the bugs.

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: seems to work

I'm pushing this to the try server now. If it passes, then I'll request review -- I'll tackle the cleanup I mention in the XXX comments in other bugs.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 408388 [details] [diff] [review]
seems to work

This "passed" on try server (three different known [orange] bugs, and some weird NSPR linking errors on several Talos boxes). I'd like to get this in now and deal with the extra cleanup in a followup patch.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: -w

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

I'm going to attach a wip that cleans it up, in part, by using the new function in bug 533600.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: wip

This patch does a little more than just clean up the control flow: it's more of a "start using the functions introduced in bug 533600" patch. I hope this isn't a problem.

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix on top of patch for bug 533600

I can also attach a diff -w if needed.

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

This was fixed as part of compartments.