Bug for tracking implmentation of the addon itself

And I pushed mac binaries and I could flash a Z3 from a Mac. This is getting cool.

(In reply to Alexandre LISSY :gerard-majax from comment #1) > And I pushed mac binaries and I could flash a Z3 from a Mac. This is getting > cool. Rebuilt the binaries as 64 bits and static. No idea why those were failing yesterday. I could flash my Z3 a dozen of times from a mac \o/

Comment on attachment 8610785 [details] Github repo Fabrice, let's formally consider the current code for reviewing. The goal is to assert a first version upon which Etienne and others will be able to build on to provide nice UX.

Comment on attachment 8610785 [details] Github repo In about.js, please get rid of all the |let deferred = promise.defer();| and use the Promise object provided by the platform. I think I did it for all the functions in https://github.com/fabricedesre/b2g-installer/blob/master/addon/about.js It would be nice to document what the promises resolve to because some functions are a bit hard to follow. For instance, dealWithBlobFree() needs some refactoring... Also, all the DOM stuff needs to go away from this file, but I guess we can let that up to Etienne.

(In reply to Fabrice Desré [:fabrice] from comment #4) > Comment on attachment 8610785 [details] > Github repo > > In about.js, please get rid of all the |let deferred = promise.defer();| and > use the Promise object provided by the platform. I think I did it for all > the functions in > https://github.com/fabricedesre/b2g-installer/blob/master/addon/about.js > It would be nice to document what the promises resolve to because some > functions are a bit hard to follow. For instance, dealWithBlobFree() needs > some refactoring... Done for both. I agree the code was not very very easy to follow. That should be better now. > > Also, all the DOM stuff needs to go away from this file, but I guess we can > let that up to Etienne. So about.js should only be dealing with UI and we would sendAsyncMessage() with main process for all the real logic?

Comment on attachment 8610785 [details] Github repo That should be better looking. I have reworked all the promises to make use of only platform Promise object. I have also reworked the nasty dealWithBlobFree() to make it easier to read, track, and hack. I also added a small option for the flashing that allows one to keep the userdata partition untouched if: - he wants - the device already runs B2G That way people will be able to reflash a new B2G base image while keeping their data. Finally, I've also identified what may have been your issue while testing when you got images that were not booting past the SONY logo. Turns out that when we |adb root| adb gets disconnected and comes back, and that may have messed up with blobs retrieval. At least, I did reproduce your issue twice (over 20 flashes maybe, but not all from scratch) and each time the symptoms were the same: - not passing the SONY logo - rebooting back - in the temp directory's the boot.img and recovery.img were smaller than expected - in the temp directory's storing blobs locally, most of the one we pull at first were missing I've added a not so nice setTimeout() of 5 secs to get rid of this. So far, not reproduced. All those changes are done on top of the first commit you reviewed to ease tracking.

(In reply to Alexandre LISSY :gerard-majax from comment #5) > So about.js should only be dealing with UI and we would sendAsyncMessage() > with main process for all the real logic? No, all the logic could move in some backend.js and the front end in about.js/frontend.js. Just see that as a library, but don't use the message manager as an interface.

Bug 1026945 will impact us as it will make stop ADBHelper to be always-on. We will need to trigger it ourselves.

Can you upload it as a patch or do a PR? I can't let comments anywhere currently.

So it's just the same code but all squashed into one commit, and attached on Bugzilla, that should make it easier to review :)

Comment on attachment 8615531 [details] [diff] [review] Provide an addon to insall B2G onto a supported device Review of attachment 8615531 [details] [diff] [review]: ----------------------------------------------------------------- Apart from the nits, what I'd really like to see improved in about.js : - document the method parameters. - maybe encapsulate all these global methods... I feel that we'll refactor anyway when doing the real front-end. ::: Makefile @@ +1,5 @@ > +FILES=about.css about.js about.xhtml bootstrap.js imaging_tools.js main.js subprocess.js chrome.manifest > +ADDON_VERSION=0.1 > +XPI_NAME=b2g-installer-$(ADDON_VERSION) > + > +XPIS = $(XPI_NAME)-win32.xpi $(XPI_NAME)-linux.xpi $(XPI_NAME)-linux64.xpi $(XPI_NAME)-mac64.xpi I think we should rather do a single xpi instead of one per platform. Also, do we really support windows? ::: about.js @@ +33,5 @@ > +const kExpectedBlobFreeContent = [ > + kBlobFree, kBlobsInject, kCmdlineFs, kDevicesJson, kDeviceRecovery > +]; > + > +const kB2GInstallerTmp = "/tmp/b2g-installer/"; don't hardcode that, use https://github.com/fabricedesre/b2g-installer/blob/master/addon/about.js#L41 @@ +110,5 @@ > + let _src = src[0] === "/" ? src.slice(1) : src; > + let f = new FileUtils.File(OS.Path.join(blobsDir.path, _src)); > + currentBlob++; > + > + device.pull(src, f.path).then((res) => { here and everywhere else: then(res => {... when there is a single argument. @@ +928,5 @@ > + > +addEventListener("load", function load() { > + removeEventListener("load", load, false); > + > + // getAllDevices(); nit: remove ::: bootstrap.js @@ +10,5 @@ > + > +Cu.import("resource://gre/modules/Services.jsm"); > + > +const REASON = [ 'unknown', 'startup', 'shutdown', 'enable', 'disable', > + 'install', 'uninstall', 'upgrade', 'downgrade' ]; Be consistent with double quotes for strings. @@ +43,5 @@ > + > + let uri = registerAddonResourceHandler(data); > + > + let loaderModule = > + Cu.import('resource://gre/modules/commonjs/toolkit/loader.js').Loader; here too ::: main.js @@ +39,5 @@ > +// Proper execution of the full process depends on some bugs fixed: > +// - Bug 1059081: TCPSocket crash in nsMultiplexInputStream::ReadSegments on > +// "Socket Thread" due to apparent racey use of > +// nsMultiplexInputStream > +// - Bug 1164290: ZipUtils.extractFiles() breaks with symbolic links they are both fixed, no need for this comment anymore ::: template-install.rdf @@ +18,5 @@ > + <!-- Firefox --> > + <em:targetApplication> > + <Description> > + <em:id>{ec8030f7-c20a-464f-9b0e-13a3a9e97384}</em:id> > + <em:minVersion>33.0</em:minVersion> I don't think that's the right minVersion

This should address all the comments

Rebased everything into one first commit: https://github.com/lissyx/b2g-installer/commit/dd3b28f5733ee65607fcdc01501632ada0991653