Post Bug 1220476, we can skip installing an Android NDK during |mach bootstrap| for front-end (--disable-compile-environment, |mach artifact| based) builds entirely. This ticket tracks offering such build configurations during |mach bootstrap|, not installing the NDK for Android builds, and printing an appropriate mozconfig for this configuration. Bug 1207888 and Bug 1207890 block doing this for Desktop builds.

This is a [good next bug] for a Python developer.

On Sat, Nov 7, 2015 at 12:40 PM, Harshit Bansal <harshitbansal2015@gmail.com> wrote: > > Proposed message for mozconfig file: > > > To build Firefox For Android, paste the lines between the chevrons > (>>> and <<<) into your mozconfig file: > > <<< > # Build Firefox for Android: > ac_add_options --enable-application=mobile/android > ac_add_options --target=arm-linux-androideabi > > # With the following Android SDK and NDK: > ac_add_options --with-android-sdk={PATH TO SDK} > ac_add_options --with-android-ndk={PATH TO NDK} > > #OBJ DIRECTORY Let's keep the conversational style of the other messages, like: # And write build outputs into: mk_add_options... > > mk_add_options MOZ_OBJDIR = {PATH TO OBJ DIR} > >>> > > For front-end builds, we don't need to build C++ libraries(which > consume most of the build time). > To enable these settings, paste the following lines between the > chevrons(instead of the above mentioned lines) in your mozconfig file: > > <<< > #Build Firefox for Android: > ac_add_options -- enable-application=mobile/android > ac_add_options --target=arm-linux-androideabi > ac_add_options --disable-compile-environment > > # With the following Android SDK: > ac_add_options --with-android-sdk={PATH TO SDK} > > #OBJ DIRECTORY > mk_add_options MOZ_OBJDIR = {PATH TO FRONT-END OBJ DIR} > >>> > > Note : Remember to replace {..} with actual PATHS, while doing so > remember to use $HOME instead of ~ as ~ will not get expanded.Also, if > you don't have Android SDK and NDK pre-installed, then remove those > corresponding options. These will be automatically downloaded and > installed for you. Rather than offering choices when you show the mozconfig, let's add a choice after you select "2. Firefox for Android", like: """ Do you want to modify just Firefox for Android, or do you want to modify the Gecko platform? The Firefox for Android front-end is built using Java, the Android Platform SDK, JavaScript, HTML, and CSS. If you want to work on the look-and-feel of Firefox for Android, this is what you want. Firefox for Android is built on top of the Gecko technology platform. Gecko is Mozilla's web rendering engine, similar to Edge, Blink, and WebKit. Gecko is implemented in C++ and JavaScript. If you want to work on web rendering, this is what you want. If you don't know what you want, start with just the Firefox for Android front-end -- your builds will be much shorter than if you build Gecko as well. But don't worry! You can always switch configurations later. 1. Just the Firefox for Android front-end front-end ** Java and JavaScript ** short builds ** use IntelliJ or Android Studio 2. The Gecko platform and Firefox for Android platform ** C++ ** long builds ** use your text editor """ If the user selects 1, then don't install the Android NDK at all, and offer only the --disable-compile-environment mozconfig, with object directory 'objdir-frontend'. If the user selects 2, then do install the NDK, and offer the existing mozconfig with object directory 'objdir-droid'.

I know it took a very long time than it should take but now I think it's finally ready!!

Comment on attachment 8684960 [details] [diff] [review] fourth.patch Review of attachment 8684960 [details] [diff] [review]: ----------------------------------------------------------------- The messages are good, but you're installing the NDK first, and then asking if it should be used. The NDK is about 500Mb, so we don't want to install it at all if we're not going to use it. (People on bad networks will appreciate the savings.) So look at the code around https://dxr.mozilla.org/mozilla-central/source/python/mozboot/mozboot/bootstrap.py#123 and add a second choice *before* doing the installations. You'll need to ferry the information around. It might be easier to add a *new* application type -- mobile_android_artifact_mode, say -- that knows what to do. I expect you will want to replace https://dxr.mozilla.org/mozilla-central/source/python/mozboot/mozboot/osx.py#370 as well. ::: python/mozboot/mozboot/base.py @@ +71,5 @@ > pythonz -- https://github.com/saghul/pythonz > official installers -- http://www.python.org/ > ''' > > +MESSAGE = ''' These are very Android specific, so let's fold them into android.py. Give them nice long names, too: ANDROID_ARTIFACT_MODE_MESSAGE @@ +88,5 @@ > + platform ** C++ ** long builds ** use your text editor > + > +''' > + > +MOZCONFIG1 = ''' ANDROID_ARTIFACT_MODE_MOZCONFIG @@ +112,5 @@ > + > +Note : Remember to replace {..} with actual PATHS, while doing so remember to use $HOME instead of ~ as ~ will not get expanded.Also, if you don't have Android SDK pre-installed, then remove those corresponding options. These will be automatically downloaded and installed for you. > +''' > + > +MOZCONFIG2 = ''' ANDROID_MOZCONFIG @@ +203,5 @@ > + if ch == 1 : > + print(MOZCONFIG1) > + else : > + print(MOZCONFIG2) > + nit: delete trailing whitespace.

Need some clarification on how to do this : ""add a second choice *before* doing the installations. You'll need to ferry the information around. It might be easier to add a *new* application type -- mobile_android_artifact_mode, say -- that knows what to do.""

(In reply to Harshit Bansal from comment #5) > Need some clarification on how to do this : > ""add a second choice *before* doing the installations. You'll need to > ferry the information around. It might be easier to add a *new* application > type -- mobile_android_artifact_mode, say -- that knows what to do."" If you look at https://dxr.mozilla.org/mozilla-central/source/python/mozboot/mozboot/bootstrap.py#30, you'll see a map of application name (Firefox for Android) to identifier (mobile_android). We then use that identifier to call the right functions, see: https://dxr.mozilla.org/mozilla-central/source/python/mozboot/mozboot/bootstrap.py#137 and https://dxr.mozilla.org/mozilla-central/source/python/mozboot/mozboot/bootstrap.py#145. So you might add another application name (Firefox for Android, artifact mode) and another identifier (mobile_android_artifact_mode), and the appropriate functions. That would implement this choice. Then we could work on showing better messaging to explain the choices. Alternately, you could add another prompt *inside* the mobile_android logic branches, asking whether to build in artifact mode. I think I prefer the earlier approach (new application name and identifier) because we're going to want to support this for Desktop (non-Android) builds eventually.

Comment on attachment 8684960 [details] [diff] [review] fourth.patch Review of attachment 8684960 [details] [diff] [review]: ----------------------------------------------------------------- Harshit, mobile team doesn't use the checkin flag. After this has r+, I'll either land it myself, or we'll use the checkin-needed whiteboard flag for the sheriffs to land for us. Thanks!

Kindly review the attached patch!

Kindly review the patch! It's blocking the BUG 1221253.

Comment on attachment 8688579 [details] [diff] [review] fourth.patch Review of attachment 8688579 [details] [diff] [review]: ----------------------------------------------------------------- Harshit -- sorry for the delayed review. Lots to do and I'm behind on my queue. This patch is looking much better! I see you've added a flag to `ensure_android_sdk_and_ndk`, and then duplicated the `install_mobile_android*packages` functions. I think this is reversed; you want the flag to be at the `install_mobile_android*` layer. So: * split `ensure_android_sdk_and_ndk` into `ensure_android_sdk` and `ensure_android_ndk`; * make an `install_mobile_android` function that takes the flag and conditionally does the android_sdk and android_ndk things; * use the helper function above in the two `install_mobile_android*` functions. Great progress! Sorry again for the delay. ::: python/mozboot/mozboot/android.py @@ +103,5 @@ > +ac_add_options --enable-application=mobile/android > +ac_add_options --target=arm-linux-androideabi > +--disable-compile-environment > + > +# With the following Android SDK : nit: "SDK:". @@ +106,5 @@ > + > +# With the following Android SDK : > +ac_add_options --with-android-sdk={PATH TO SDK} > + > +#AND WRITE BUILD OUTPUT INTO nit: keep the style of the rest of the message: # And write build output into: @@ +249,5 @@ > > # It's not particularyl bad to overwrite the NDK toolchain, but it does take > # a while to unpack, so let's avoid the disk activity if possible. The SDK > # may prompt about licensing, so we do this first. > + if not artifact_mode : nit: no space before :. @@ +303,2 @@ > > def android_ndk_url(os_name, ver='r10e') : nit: remove spaces before : throughout. @@ +307,5 @@ > > if maxsize > 2**32 : > return (base_url+ver+'-'+os_name+'-x86_64.bin') > else : > + return (base_url+ver+'-'+os_name+'-x86.bin') whatchanged here? ::: python/mozboot/mozboot/base.py @@ +124,5 @@ > % __name__) > > + def install_mobile_android_artifact_mode_packages(self): > + ''' > + Install packages required to build Firefox for Android (application "for Android Artifact Mode" @@ +128,5 @@ > + Install packages required to build Firefox for Android (application > + 'mobile/android', also known as Fennec). > + ''' > + raise NotImplementedError('Cannot bootstrap Firefox for Android: ' > + '%s does not yet implement install_mobile_android_packages()' install_mobile_android_artifact_mode_packages ::: python/mozboot/mozboot/bootstrap.py @@ +23,5 @@ > APPLICATION_CHOICE = ''' > Please choose the version of Firefox you want to build: > %s > + > +Note: (For Firefox for Android) This note is similar to the `android.ARTIFACT_MODE_MESSAGE`. Should that be moved here and removed from the `android` module? @@ +27,5 @@ > +Note: (For Firefox for Android) > + > +Firefox for Android is built on top of the Gecko technology platform. Gecko is Mozilla's web rendering engine, similar to Edge, Blink, and WebKit. Gecko is implemented in C++ and JavaScript. If you want to work on web rendering, this is what you want. > + > +The Firefox for Android Front-End(ARTIFACT MODE) is built using Java, the Android Platform SDK, JavaScript, HTML, and CSS. If you want to work on the look-and-feel of Firefox for Android, this is what you want. drop "(ARTIFACT MODE). @@ +29,5 @@ > +Firefox for Android is built on top of the Gecko technology platform. Gecko is Mozilla's web rendering engine, similar to Edge, Blink, and WebKit. Gecko is implemented in C++ and JavaScript. If you want to work on web rendering, this is what you want. > + > +The Firefox for Android Front-End(ARTIFACT MODE) is built using Java, the Android Platform SDK, JavaScript, HTML, and CSS. If you want to work on the look-and-feel of Firefox for Android, this is what you want. > + > +If you don't know what you want, start with just the Firefox for Android Artifact Mode your builds will be much shorter than if you build Gecko as well. But don't worry! You can always switch configurations later. "Mode. Your builds..." @@ +149,5 @@ > > self.instance.ensure_mercurial_modern() > self.instance.ensure_python_modern() > > + # Like 'suggest_browser_mozconfig' or 'suggest_mobile_android_mozconfig'. Why re-order this and the print statement below?

Partially tested on Linux. How to check on Mac OS?

Comment on attachment 8692635 [details] [diff] [review] fourth.patch Review of attachment 8692635 [details] [diff] [review]: ----------------------------------------------------------------- This is better, but I'm frustrated that many of the nits I asked to be addressed have not been addressed in this iteration. I am sure it's frustrating to have a bunch of nits about English grammar, so let's plan to have me do a final pass before landing to clean those little things up. I have some substantive issues, mostly around duplication of the existing functions. Let's get those down next. ::: python/mozboot/mozboot/android.py @@ +94,5 @@ > +mk_add_options MOZ_OBJDIR = ./objdir-frontend > + > +>>> > + > +Note: Remember to replace {..} with actual PATHS, while doing so remember to use $HOME instead of ~ as ~ will not get expanded.Also, if you don't have Android SDK pre-installed, then remove those corresponding options. These will be automatically downloaded and installed for you. I'm quite sure I actually printed the paths with substitutions here, so we shouldn't need this. Also, I asked for some changes to the text above last review. @@ +225,5 @@ > > +def ensure_android_sdk(path, sdk_path, sdk_url, artifact_mode=False): > + ''' > + Ensure the Android SDK is found at the given path. If not, fetch > + and unpack the SDKfrom the given URLs into |path|. nit: SDK from. @@ +252,5 @@ > else: > install_mobile_android_sdk_or_ndk(ndk_url, path) > > +def install_mobile_android(artifact_mode, plat): > + '''Ensures Android SDK AND NDK(if not in Artifact Mode) are installed''' nit: SDK and NDK (if appropriate) are installed. @@ +254,5 @@ > > +def install_mobile_android(artifact_mode, plat): > + '''Ensures Android SDK AND NDK(if not in Artifact Mode) are installed''' > + > + path =os.environ.get('MOZBUILD_STATE_PATH', os.path.expanduser(os.path.join('~', '.mozbuild'))) nit: space after =, no trailing whitespace at end of line. @@ +262,5 @@ > + > + ndk_path = os.environ.get('ANDROID_NDK_HOME', os.path.join(path, 'android-ndk-r10e')) > + ndk_url = android_ndk_url(plat) > + > + #The SDK may prompt about licensing, so we are installing NDK first. nit: space after #. "The SDK may prompt about licensing, so we install the NDK first." @@ +269,5 @@ > + > + ensure_android_sdk(path, sdk_path, sdk_url) > + > + if artifact_mode: > + return sdk_path In general, functions that return different types are a bad idea. Let's either return `(None, sdk_path)`, or fold the `os.environ` checking into the `ensure_android_{sdk,ndk}` functions themselves. @@ +318,5 @@ > arch = 'x86_64' > else : > arch = 'x86' > > + return (base_url+ver+'-'+os_name+'-'+arch+'.bin') The fixes I landed to fix the earlier bustage changed this code, so this needs to be rebased on top of that. Follow the style from those changes, please. ::: python/mozboot/mozboot/archlinux.py @@ +128,5 @@ > + # 1. System packages. > + # 2. Android SDK. > + # 3. Android packages. > + > + # 1. This is hard to believe, but the Android SDK binaries are 32-bit This duplication is unfortunate, and I don't think it can be correct. Where are you using the new helper functions in `android`? ::: python/mozboot/mozboot/base.py @@ -71,5 @@ > pythonz -- https://github.com/saghul/pythonz > official installers -- http://www.python.org/ > ''' > > - nit: revert this. ::: python/mozboot/mozboot/bootstrap.py @@ +45,5 @@ > # no OrderedDict) > APPLICATIONS = dict( > desktop=APPLICATIONS_LIST[0], > android=APPLICATIONS_LIST[1], > + android_artifact=APPLICATIONS_LIST[2], Shouldn't this be `android_artifact_mode`? I'll test this locally and read more to understand this. @@ +103,5 @@ > else: > raise NotImplementedError('Bootstrap support for this Linux ' > 'distro not yet available.') > > + nit: revert this. ::: python/mozboot/mozboot/debian.py @@ +139,5 @@ > + # 1. System packages. > + # 2. Android SDK. > + # 3. Android packages. > + > + # 1. This is hard to believe, but the Android SDK binaries are 32-bit This and the existing are the same save for a single flag. Extract them into a helper, and pass the flag in. In general, Don't Repeat Yourself (https://en.wikipedia.org/wiki/Don't_repeat_yourself).

(In reply to Nick Alexander :nalexander from comment #12) > Comment on attachment 8692635 [details] [diff] [review] > fourth.patch > > Review of attachment 8692635 [details] [diff] [review]: > ----------------------------------------------------------------- > Sorry for a late reply! Currently my exams are going on. I will resume as soon as they are over. > This is better, but I'm frustrated that many of the nits I asked to be > addressed have not been addressed in this iteration. I am sure it's > frustrating to have a bunch of nits about English grammar, so let's plan to > have me do a final pass before landing to clean those little things up. I > have some substantive issues, mostly around duplication of the existing > functions. Let's get those down next. Can you point me out some good document related to Mozilla's Python coding style. > > ::: python/mozboot/mozboot/android.py > @@ +94,5 @@ > > +mk_add_options MOZ_OBJDIR = ./objdir-frontend > > + > > +>>> > > + > > +Note: Remember to replace {..} with actual PATHS, while doing so remember to use $HOME instead of ~ as ~ will not get expanded.Also, if you don't have Android SDK pre-installed, then remove those corresponding options. These will be automatically downloaded and installed for you. > > I'm quite sure I actually printed the paths with substitutions here, so we > shouldn't need this. Also, I asked for some changes to the text above last > review. > I think in the previous review you asked for removing space before':' and adding space after '#' which I think I have already done in this patch.Can you please clarify this one? Also, I think that we can't print a static path here as it may be different if the user has a pre-installed SDK. Am I right? > @@ +225,5 @@ > > > > +def ensure_android_sdk(path, sdk_path, sdk_url, artifact_mode=False): > > + ''' > > + Ensure the Android SDK is found at the given path. If not, fetch > > + and unpack the SDKfrom the given URLs into |path|. > > nit: SDK from. > > @@ +252,5 @@ > > else: > > install_mobile_android_sdk_or_ndk(ndk_url, path) > > > > +def install_mobile_android(artifact_mode, plat): > > + '''Ensures Android SDK AND NDK(if not in Artifact Mode) are installed''' > > nit: SDK and NDK (if appropriate) are installed. > > @@ +254,5 @@ > > > > +def install_mobile_android(artifact_mode, plat): > > + '''Ensures Android SDK AND NDK(if not in Artifact Mode) are installed''' > > + > > + path =os.environ.get('MOZBUILD_STATE_PATH', os.path.expanduser(os.path.join('~', '.mozbuild'))) > > nit: space after =, no trailing whitespace at end of line. > > @@ +262,5 @@ > > + > > + ndk_path = os.environ.get('ANDROID_NDK_HOME', os.path.join(path, 'android-ndk-r10e')) > > + ndk_url = android_ndk_url(plat) > > + > > + #The SDK may prompt about licensing, so we are installing NDK first. > > nit: space after #. > > "The SDK may prompt about licensing, so we install the NDK first." > > @@ +269,5 @@ > > + > > + ensure_android_sdk(path, sdk_path, sdk_url) > > + > > + if artifact_mode: > > + return sdk_path > > In general, functions that return different types are a bad idea. Let's > either return `(None, sdk_path)`, or fold the `os.environ` checking into the > `ensure_android_{sdk,ndk}` functions themselves. > > @@ +318,5 @@ > > arch = 'x86_64' > > else : > > arch = 'x86' > > > > + return (base_url+ver+'-'+os_name+'-'+arch+'.bin') > > The fixes I landed to fix the earlier bustage changed this code, so this > needs to be rebased on top of that. Follow the style from those changes, > please. > > ::: python/mozboot/mozboot/archlinux.py > @@ +128,5 @@ > > + # 1. System packages. > > + # 2. Android SDK. > > + # 3. Android packages. > > + > > + # 1. This is hard to believe, but the Android SDK binaries are 32-bit > > This duplication is unfortunate, and I don't think it can be correct. Where > are you using the new helper functions in `android`? I think its 'archlinux.py'. I will add them in the next patch. Don't know how I forgot about it. > > ::: python/mozboot/mozboot/base.py > @@ -71,5 @@ > > pythonz -- https://github.com/saghul/pythonz > > official installers -- http://www.python.org/ > > ''' > > > > - > > nit: revert this. > > ::: python/mozboot/mozboot/bootstrap.py > @@ +45,5 @@ > > # no OrderedDict) > > APPLICATIONS = dict( > > desktop=APPLICATIONS_LIST[0], > > android=APPLICATIONS_LIST[1], > > + android_artifact=APPLICATIONS_LIST[2], > > Shouldn't this be `android_artifact_mode`? I'll test this locally and read > more to understand this. > > @@ +103,5 @@ > > else: > > raise NotImplementedError('Bootstrap support for this Linux ' > > 'distro not yet available.') > > > > + > > nit: revert this. > > ::: python/mozboot/mozboot/debian.py > @@ +139,5 @@ > > + # 1. System packages. > > + # 2. Android SDK. > > + # 3. Android packages. > > + > > + # 1. This is hard to believe, but the Android SDK binaries are 32-bit > > This and the existing are the same save for a single flag. Extract them > into a helper, and pass the flag in. In general, Don't Repeat Yourself > (https://en.wikipedia.org/wiki/Don't_repeat_yourself). Okay, I will add all this in 'install_mobile_android' helper function.

Harshit, I would like to work on this if you are not.

I was unable to work on it for sometime as my exams were going on but now they are over and I am ready to continue again!!

Added option to build firefox for android artifact mode Works only in OSX using Homebrew as the package manager

Comment on attachment 8717735 [details] [diff] [review] Build Firefox for Android Artifact mode Review of attachment 8717735 [details] [diff] [review]: ----------------------------------------------------------------- Hey! Sorry for the delayed review. This looks really good! If you could do the work for Linux that would be great. Also, we'll need to message people about what "artifact mode" means, and link to the documentation. Some of that's in the earlier patch, I think. ::: python/mozboot/mozboot/osx.py @@ +366,5 @@ > self.ndk_url = android.android_ndk_url('darwin') > else: > raise Exception('You need a 64-bit version of Mac OS X to build Firefox for Android.') > > + self.artifact_mode = artifact_mode No need to store this -- you don't use it as a member again, do you?

> ::: python/mozboot/mozboot/osx.py > @@ +366,5 @@ > > self.ndk_url = android.android_ndk_url('darwin') > > else: > > raise Exception('You need a 64-bit version of Mac OS X to build Firefox for Android.') > > > > + self.artifact_mode = artifact_mode > > No need to store this -- you don't use it as a member again, do you? Yes, I am using it again in the suggest_%s_mozconfig functions.

Comment on attachment 8719973 [details] [diff] [review] Offer Firefox for Android Artifact Mode build in |mach bootstrap| Review of attachment 8719973 [details] [diff] [review]: ----------------------------------------------------------------- Sambuddha - first, sorry for the delayed reply. This is looking good, but I have two large comments. The first is that changes to ``android.py`` seem to have been lost. Is this on top of your previous patch or a refreshed patch? (It looks refreshed.) If it's on top, label them Part 1 and Part 2, etc, please. Second, I'd really like to get rid of the ``artifact_mode`` member. You're declaring a new "application": there's mobile_android and now mobile_android_artifact_mode. You can pass the artifact_mode=True flag around in there; no need to have state. In general, the more state you can have flowing through function parameters and the less encapsulated in objects, the easier it is to reason about and maintain your code. Thanks!

Review commit: https://reviewboard.mozilla.org/r/36145/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/36145/

(In reply to Nick Alexander :nalexander from comment #20) > Comment on attachment 8719973 [details] [diff] [review] > Offer Firefox for Android Artifact Mode build in |mach bootstrap| > > Review of attachment 8719973 [details] [diff] [review]: > ----------------------------------------------------------------- > > Sambuddha - first, sorry for the delayed reply. > > This is looking good, but I have two large comments. The first is that > changes to ``android.py`` seem to have been lost. Is this on top of your > previous patch or a refreshed patch? (It looks refreshed.) If it's on top, > label them Part 1 and Part 2, etc, please. I have uploaded a refreshed patch to https://reviewboard.mozilla.org/r/36145/diff/#index_header > > Second, I'd really like to get rid of the ``artifact_mode`` member. You're > declaring a new "application": there's mobile_android and now > mobile_android_artifact_mode. You can pass the artifact_mode=True flag > around in there; no need to have state. In general, the more state you can > have flowing through function parameters and the less encapsulated in > objects, the easier it is to reason about and maintain your code. In the new patch, I have got rid of the ``artifact_mode`` member and am passing the artifact_mode=True flag to ``install_mobile_android_artifact_mode_packages`` and ``suggest_mobile_android_artifact_mode_mozconfig``.

Comment on attachment 8722628 [details] MozReview Request: Bug 1221200 - Offer Firefox for Android Artifact Mode build in |mach bootstrap| r?nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36145/diff/1-2/

Comment on attachment 8722628 [details] MozReview Request: Bug 1221200 - Offer Firefox for Android Artifact Mode build in |mach bootstrap| r?nalexander https://reviewboard.mozilla.org/r/36145/#review33837 Sambuddha Basu -- This patch looks great! I'm going to apply it and test it locally, and I will land it. I want to format and update the help messages just a little bit. Thanks for your patience with my slow review process. Well done! I haven't filed a ticket for it yet, but I would like to do something like this for *all* builds, not just Firefox for Android. If you're interested in extending this, please file it. Otherwise, you seem to have very strong Python skills. Are you familiar with the build system? I have a couple of tickets to improve the Fennec build system. One is purging some unused and crufty code; it's straight-forward but requires an attention to detail. The other is to improve some preprocessing and simple `mobile/android/base/Makefile.in`. Let me know if you're interested and I can file and we can talk. Thanks again!

https://hg.mozilla.org/integration/fx-team/rev/7280ed7ecd34 https://hg.mozilla.org/integration/fx-team/rev/8d604d4a13f3

Sambuddha -- landed with minor formatting changes! Well done!

https://hg.mozilla.org/mozilla-central/rev/7280ed7ecd34 https://hg.mozilla.org/mozilla-central/rev/8d604d4a13f3