Closed Bug 1310628 Opened 6 years ago Closed 6 years ago

Expose addon.js methods in GeckoDriver

Categories

(Testing :: Marionette, defect)

Version 3
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1298025

People

(Reporter: whimboo, Unassigned, Mentored)

Details

(Keywords: good-first-bug, Whiteboard: [lang=js][lang=py])

The following commit added the addon.js module to the Marionette server but actually didn't expose the contained methods on the GeckoDriver class.

https://hg.mozilla.org/releases/mozilla-release/rev/2591ed8e9b67

So after talking to Andreas we would need two methods in the GeckoDriver class, one for installing and another one for uninstalling an addon. Both would have to be added to the GeckoDriver.prototype.commands list in driver.js:

https://dxr.mozilla.org/mozilla-central/rev/9079d167112122805f99f57bb8856e1b1675af0f/testing/marionette/driver.js#2697

Andreas, do the following two method names make sense?

* installAddon
* uninstallAddon


Once that has been done we most likely want to get rid of the code in addons.py and simply forward the commands to the appropriate server end-points.

Maybe we can make this a mentored project?
Flags: needinfo?(ato)
Talked to Andreas on IRC and he proposed to use a namespace prefix:

> whimboo: Eventually I want to give the Marionette protocol proper namespacing so do you mind calling the commands Marionette:l10n:<name>?

So the names should look like:

* Marionette.addon.install
* Marionette.addon.uninstall
Good idea.  Made this a mentored bug.
Mentor: ato
Flags: needinfo?(ato)
Whiteboard: [good first bug][lang=js]
Whiteboard: [good first bug][lang=js] → [good first bug][lang=js][lang=python]
Whiteboard: [good first bug][lang=js][lang=python] → [good first bug][lang=js][lang=py]
Whiteboard: [good first bug][lang=js][lang=py] → [lang=js][lang=py]
Keywords: access
Looks like something I'll be able to work on. Can someone help me with this?
What do you need help with?
(In reply to Andreas Tolfsen ‹:ato› from comment #4)
> What do you need help with?

So here's what I have gathered from whimboo's comments:

1. Add two methods to GeckoDriver.prototype.commands list in driver.js:
    * "installAddon": Marionette.addon.install
    * "uninstallAddon": Marionette.addon.uninstall
Is this good?

2. Remove the code related to addons in addon.py. Now my doubt here is that addon.py contains only two methods install and uninstall. Removing those methods would virtually leave nothing in the file. So would it be safe to remove the complete file? 

3. I didn't understand whimboo when he said : "simply forward the commands to the appropriate server end-points."
(In reply to Dhanesh Sabane [:dhanesh95] (UTC+5:30) from comment #5)
> 1. Add two methods to GeckoDriver.prototype.commands list in driver.js:
>     * "installAddon": Marionette.addon.install
>     * "uninstallAddon": Marionette.addon.uninstall
> Is this good?

You need to implement two “wrapper functions” in testing/marionette/driver.js that deserialises the command body and passes it on to testing/marionette/addon.js.  These could be called GeckoDriver#installAddon and GeckoDriver#uninstallAddon, which are the functions you need to expose in GeckoDriver#commands.

When you expose them, please use a namespace such as this:

    {
      …
      "addon:install": GeckoDriver.installAddon,
      "addon:uninstall": GeckoDriver.uninstallAddon,
      …
    }

The implementation of GeckoDriver#installAddon should look something like this:

    GeckoDriver.prototype.installAddon = function(cmd, resp) {
      let {path, temporary} = cmd.parameters;
      if (typeof path == "undefined" || typeof temporar == "undefined") {
        throw new InvalidArgumentError();
      }
      
      let addonId = yield this.addon.install(path, temporary);
      return {id: addonId};
    };

> 2. Remove the code related to addons in addon.py. Now my doubt here is that
> addon.py contains only two methods install and uninstall. Removing those
> methods would virtually leave nothing in the file. So would it be safe to
> remove the complete file? 

We should leave the file as it is.  The install and uninstall methods there needs to call _send_message("Marionette:addon:install", …) with the appropriate body that is accepted by the commands you are exposing from the server.
(In reply to Andreas Tolfsen ‹:ato› from comment #6)
>       "addon:install": GeckoDriver.installAddon,
>       "addon:uninstall": GeckoDriver.uninstallAddon,

Of course I meant GeckoDriver.prototype.installAddon here.
I have read the content so far posted here and this is what i think the task is.
1) import the addon.js file to get the install and uninstall methods
2) Update the GeckoDriver.prototype.commands object with GeckoDriver.prototype.installAddon and GeckoDriver.prototype.uninstallAddon and their respective function is to be implemented in which ill just pass the path, temporary params.

I have few doubts;
1) From reading he addon.js i can see that both the functions are es6 promise so what should i return if the promise is rejected or failed.
So shouldnt the function wrapper where the promise is handled be something like this?

    this.addon.install(path, temporary) 
      .then(function(data){
         //do somthing with data i.e ge the addonID
       })
       .catch(function(err){
         //do something specific to error
       });

2) So i can just use Components.utils.import("resource://path-to-file") to get the addon.js module?
if so what is the path?
(In reply to Nishanth Mane [:nishu-tryinghard] from comment #8)
> 1) From reading he addon.js i can see that both the functions are es6
> promise so what should i return if the promise is rejected or failed.
> So shouldnt the function wrapper where the promise is handled be something
> like this?
> 
>     this.addon.install(path, temporary) 
>       .then(function(data){
>          //do somthing with data i.e ge the addonID
>        })
>        .catch(function(err){
>          //do something specific to error
>        });

Promises are automatically resolved in the dispatcher if they are not yielded in the commands themselves: https://github.com/mozilla/gecko-dev/blob/master/testing/marionette/dispatcher.js#L123

So you can implement it this way:

    GeckoDriver.prototype.installAddon = function (cmd, resp) {
      let {path, temporary} = cmd.parameters;
      return addon.install(path, temporary);
    });


> 2) So i can just use Components.utils.import("resource://path-to-file") to
> get the addon.js module?
> if so what is the path?

Yes:

    Cu.import("chrome://marionette/content/addon.js");
I wanted to pick up this one, but if I look at the source changes at https://github.com/mozilla/gecko-dev/commit/9d38145570b1160ee8a535a39995f03cdb588790, it seems that this is already quite far in progress and does not require any additional input?
That commit doesn’t expose the commands to the world: it just moves the functionality from the client to the server.
Looks like this bug is obsolete now given that the requested code has been landed via the patch on bug 1298025. I'm duping appropriately.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.