556585 - Simplify cuddlefish.js

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[Atul Varma [:atul]]

packages/jetpack-core/lib/cuddlefish.js is really hard to understand. We ought to simplify it, or at least document it with diagrams.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

As currently described, this bug is a bit vague.  It would be good to identify the specific issue(s) that we'd like to resolve with this codebase.

Comment 2

[Nickolay_Ponomarev]

> As currently described, this bug is a bit vague.
Myk, try reading cuddlefish.js, the question of what this bug is about should go away ;-)

I'll highlight the first two major questions that arise when reading it, hoping that it will help improving/documenting it.


1) where/when cuddlefish.js is used? I found two:
  a. when setting up initial CommonJS modules system (from components/harness.js); although it's really hard to figure that out, since it's loaded via cryptic
 Cu.import(options.loader, jsm);
where options.loader (I think) eventually comes from either ENV vars set by a number of python modules or a configuration file.

  b. when a module (typically, unit tests) require() it to create a 'sandboxed loader' (see makeSandboxedLoader in the unit-test module, unfortunately also not documented)

but the code tries to handle other cases, the reason is not clear to me:
     var myURI = Components.stack.filename.split(" -> ").slice(-1)[0];
...
       // The securable-module module is at a chrome URI, so we can't
...
     // We're being loaded in a chrome window, or a web page with
     // UniversalXPConnect privileges.

2) What it does? My best guess currently is that it provides a Loader object that's not documented, which is not exactly a constructor, but is called as such :).

There's also a Loader in securable-module, which doesn't help understanding.

Of course, the general idea is clear (more or less), but to get more detailed understanding I think I'll have to try to memorize securable-module, cuddlefish users and cuddlefish.js first.

Comment 3

[Atul Varma [:atul]]

Attachment: WIP 1, gets rid of the ridonkulous CuddlefishModule class

Comment 4

[Atul Varma [:atul]]

Attachment: WIP 2, gets rid of the ridonkulous cuddlefishSandboxFactory object

Comment 5

[Atul Varma [:atul]]

Attachment: WIP 3, added optional securityPolicy interface

Comment 6

[Atul Varma [:atul]]

Attachment: WIP 4, minor refactoring of WIP 3

Comment 7

[Atul Varma [:atul]]

Myk, any idea how to go about pushing this to the repo? Nobody's reviewed the modules this patch touches, and these patches simplify said modules, which will make them easier to code review in the future...

Comment 8

[Dietrich Ayala (:dietrich)]

Given that the code is clearly functionally sound (after all the releases since it landed), I'd not hold back these changes on getting the old and harder-to-read code reviewed first. Assuming that all the tests in the SDK pass, r=me on landing it, and getting post-review on the unreviewed modules before 1.0 (which, IIRC, is a 1.0 criteria).

Comment 9

[Myk Melez [:myk] [@mykmelez]]

Atul: what Dietrich said (modulo some concern about unexpected consequences); go ahead and land 'em.

Comment 10

[Atul Varma [:atul]]

Awesome! Pushed:

Bug 556585 - Simplify cuddlefish.js
Atul Varma [:atul]
http://hg.mozilla.org/labs/jetpack-sdk/rev/c2b2d91bdb37

Comment 11

[Myk Melez [:myk] [@mykmelez]]

The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".