SummerOfCode/2013/ClearerAddonInstallation
The progress of the GSoC project Clearer Add-on Installation will be documented here. I'll also write relevant details here.
Contents
Wrapping up
GSoC 2013 just got over.
This post here will wrap up everything I've done for the project and comment on the states of all the bugs that are part of it.
- Bug 640775 - Allow a form of uninstalling add-ons installed globally by third-party applications or system administrators:
I first discussed with Mossop the methods we could use to provide this support. We decided on disabling + hiding the add-on as the form of uninstallation. My first patch didn't modify the APIs well enough to make them coherent with this new system. Also Irving was working on conflicting changes in bug 853388. So I moved on to other bugs until that was resolved. When it resolved, I submitted my next few patches that mostly kept making the new uninstallation more coherent with the existing system (no special-casing). Blair has been giving positive responses to them. I've finally brought it to a stage which I think is close to acceptable. Blair is to review it.
- Bug 879480 - Allow external processes to request add-on installs / uninstalls:
This was the most time consuming bug.
I asked Mossop for a lot clarifications on this. The first accomplishment was establishing a communication mechanism to inform Firefox about add-ons. I created 2 command line flags for this. I got Firefox to add the add-on at hand to the database (installation) and show the opt-in screen. The rest of the time I ensured things would work out fine in cases where copies of a single add-on are located in different locations. Unfortunately I took too long to receive feedback on my first patch. When Mossop got to it, he said I was heading in the right direction and pointed at some issues in my patch. I addressed them with another patch which hopefully someone will get to it soon.
The other small part of this bug, uninstallation, is dependent on the first bug in this list. However I did write code that will work when that bug gets fixed. (It's just that when there is no way to uninstall globally installed add-ons there is no meaning in making it happen by an external process) I also implemented a notification box UI that tells the user about bugs that got uninstalled and need a restart. The UX review I requested hasn't been attended to, yet.
- Bug 769495 - about:newaddon should scale better with multiple add-ons
I first made a post here with a mockup design asking for others' opinions. When Jorge liked my idea, I went on implement it. It was fairly straightforward. I posted screens and the patch on the bug and asked for ux-review. No one has gotten to it yet.
Extra bugs: I asked Jorge to assign me to other bugs when all the 3 main bugs were stalled, awaiting responses from other people.
- Bug 897735 - Support regular expression filters for metadata in extension blocks
After much discussion, we realized it is simplest to change nsIBlocklistService so that accepts Addon objects instead of their IDs. I have submitted a working patch with a test, but there is a small piece of info I need from Blair to finish it off. A needinfo flag awaits his attention.
- Bug 802434 - Support resetting preferences when disabling blocklisted add-ons
This has landed.
(All the code is attached in the bugs as patches.)
GSoC ended but I'll stick around and ensure all the bugs get resolved. I also intend to continue contributing to Mozilla as it has been an amazing learning experience.
I'm thankful to my mentor Jorge, Mossop, Blair, Kris, Irving and the whole Mozilla community for their help.
September 9 - September 17
I wrote the test Blair had asked for in bug 802434. I also have been working on bug 897735. It's mostly complete (must write a test) and I'll submit a patch soon.
As I had indicated in my tentative schedule, I had my mid-semester exams in college this week and haven't spent that much time coding.
August 29 - September 8
I've addressed the last comments in both bug 640775 and 879480 with patches. I think both the patches quite close to getting accepted. However bug 879480 won't resolve until they decide on a policy. Blair said he doesn't want to give these new flags to the vendors and in no time change the entire policy disallowing external add-on installation. So I doubt my work will get merged before the GSoC period ends.
August 20 - August 28
I spoke with Blair on IRC and got a lot of questions I had answered.
My patch to bug 640775 got some comments after Blair reviewed it. I'll address those comments with another patch in a couple of days.
About bug 897735, as Mossop suggested, we've decided to change the blocklist API to take in an add-on object instead of just the ID. I'll work on that after I'm done with the three primary bugs.
Bug 879480 depends on bug 640775 for the uninstallation part. I have made most of the suggested changes in my patch, but I'm finding it difficult to figure out what about:addons must show for an existing non-restartless add-on getting overridden by a new add-on of the same ID by the use of the flag. Since at that point the old add-on is getting disabled, but is still active, and the new one is getting installed, but isn't installed yet, I'm unsure of what the entry should look like. Blair suggested I use the add-on upgrade notification there. However, I'm unable to reproduce that upgrade notification in my computer for any use case. Perhaps I should file a bug for this.
August 11 - August 19
I confirmed with Irving that his bug wouldn't affect my bug 640775 any longer. So, I submitted a patch there. Blair will hopefully review it soon.
I spent most of the time working on bug 879480. I've addressed most of Mossop's comments and it is working fine on my computer for the best cases. I particularly have to worry about cases where an add-on is already installed somewhere and another one with the same ID gets installed somewhere else with the use the flag. processFileChanges was taking care of that before, but Mossop told me that it does more than what is necessary and has side-effects. So I have to do that stuff myself.
About bug 897735, I'm still of the opinion that changing the blocklist service API to an asynchronous one would be the only solution. But that may be because I don't fully understand how that code works. I'll look into that bug again once I'm done with these 2.
August 3 - August 10
This week some of the bugs have started to clear up.
I and Jorge decided that using an addon cache would be the best for bug 897735. But I realized that this tweaking causes some other issues. I submitted another patch to the bug and explained what the problem with that approach is. I'm thinking that making the blocklist service API asynchronous is the only way to deal with this situation. Hope we decide on that quickly.
I asked Irving about the SQLite to JSON db conversion bug. He told me it would resolve by Monday and I could start working on my dependent bug after that. He explained why a change in db revision number isn't needed to add a new property that my bug needs.
Mossop finally gave me some feedback on my patch to bug 879480. It is mostly good, and so I expect to submit a patch to it very soon. But this touches some code in the db conversion bug, so I'll have to wait till Monday.
My ux-review request in the multiple add-on opt-in screen bug hasn't been attended to, yet.
July 25 - August 2
I submitted a patch to bug 897735. I had to make a slightly inconvenient adjustment to deal with the asynchronous nature of the AddonManager API (I've explained in the bug).
I've not done much this week since I'm still waiting for responses from others. I hope bug 853388 lands soon, because I'll have quite a lot of merging to do when it does.
I also passed my mid-term evaluation. I look forward to spend the next half of GSoC as productively as this.
July 17 - July 24
The prototype opt-in screen had a few issues in design, as Jorge pointed. I fixed them and asked for ux-review on a screenshot along with the latest patch.
I didn't do much these days because all of the bugs need reviews/responses. Also, since I started early, I was taking a little break. I asked Jorge for any other bugs I could look into and now I'm assigned to bug 897735. I've understood the problem and will submit a patch soon.
July 9 - July 16
I created a prototype opt-in screen that supports multiple add-ons. It is functionally complete, but there are obviously changes needed in how it looks. I submitted a screenshot and a patch in the bug. We've left out the vendor message support here because that bug may not reach a resolution soon.
I've also been working on the uninstall aspect of bug 769495. I've used [this https://developer.mozilla.org/en-US/docs/XUL/notificationbox] notification to notify the user about a restart that maybe necessary when an add-on is uninstalled. Some feedback on my patch that works on installation would really help here because it's essentially the same kind of checking that is to be done.
I made those small changes Jorge asked for in bug 802434.
As for the global add-on removal bug, I have noted the small changes I have to make, but I still have to wait for the database conversion from sqlite to json to happen because only then can I create a new property on an add-on. The bug wouldn't be fixed right without an API that returns "uninstalled but locked" add-ons.
July 1 - July 8
My tasks were stalled because all the bugs were (with Blair to give feedback on bug 769495). So, I asked Jorge for some other bug for me to look at in the meantime and he suggested bug 802434. I worked on it and submitted a patch which Jorge is to review.
I started working on multiple add-on support for the opt-in screen though it is dependent on other bugs. I've been trying to create an XBL binding for each add-on item and make all the other necessary changes in code. I plan have it working locally on my computer while the other bugs resolve.
June 24 - June 30
I submitted a patch to bug 769495. It mostly tries to do the same checking that happens when a fresh startup occurs, only that it checks less by limiting the checking to the new add-on that is to be added. The patch however doesn't show the opt-in screen due to the reason I have mentioned along with the patch in the bug. I'm currently waiting for Blair to give me some feedback on it.
June 15 - June 23
Sorry I'm a couple of days late for my weekly update.
The patch I submitted to bug 640775 has these problems:
- Add-ons uninstalled this new way are returned by the AddonManager API because it merely hides the add-ons from the UI. I should their 'visible' property to false when they are removed so that they aren't returned.
- Some minor coding style issues.
- The changes I made conflict with those in bug 853388. We've decided that I wait till that bug is resolved because I have other bugs to attend to anyway. That bug shouldn't take long.
I continued work on bug 769495. I asked Dave Townsend for suggestions on what approach I must take. We've decided on the following approach:
- Vendors install add-ons like before (registry on Windows and copying files on others).
- They have an option of notifying an open instance of Firefox when they install the add-on, using the flag "install-global-addon" and as a parameter, the registry location where they made their entry in case of Windows or the path where the add-on is installed in other cases.
- If they do so, we check for open instances of Firefox. If there are none, we do nothing as things will fall in place when a new instance is opened.
- If there is an open instance we ask it to scan the installation directory and look for this new add-on.
- If it is found, we show the opt-in screen.
- Uninstallation-wise we are limited due to the fact that files may not be removed while Firefox is running. Only on Windows, since it is safe to remove the registry entry even while Firefox is running, we could notify the user of the removal of the add-on or the need for a restart if that is the case.
I've been able to make this feature work by exposing a new method in the AddonManager: "addInstalledAddon(<file>)" that looks for a new add-on in that location, adds it to the database and returns the id. The disk scanning I'm using is certainly not the most optimum. I'm mostly just doing the same scanning that happens on startup after adding the new add-on to the map in the DirectoryInstallLocation.
Also, I'm yet to support the registry path as parameter for Windows.
June 8 - June 14
I started writing code. I submitted a patch for bug 640775. Jorge granted the review. However, we are waiting for Blair to review it next.
I have been thinking of ways to modify the opt-in screen to allow support for multiple add-ons. I asked here for ideas and feedback on a basic design I thought of. Also, bug 879480 depends on bug 834385 and so we are waiting for a finalization there.
As for bug 769495 I was able to create an XPCOM component that recognises the flag "install-global-addon" but its functionality is yet to be added.
I wasn't updating the makefile with the new component js file I created, so changes I made weren't getting reflected until I finally realized that.
I should better understand how XPCOM components work in general (I should be able to trace the flow of information). I'm really confused by the component here: http://mxr.mozilla.org/mozilla-central/source/browser/components/shell/src/nsSetDefaultBrowser.js which is registered here http://mxr.mozilla.org/mozilla-central/source/browser/components/shell/src/nsSetDefaultBrowser.manifest#3. The component doesn't have a contract ID. When we register a component, I thought we must put the contract ID in the manifest. But they have used this contract ID: @mozilla.org/browser/default-browser-clh;1. I don't understand how they could use this to register that component.
Till June 7
I still haven't started writing any code. Since regular weekly updates are to start when coding starts, I'll just summarize what I've been doing.
We had a short meeting to get to know each other better. I found out that I have to use #addons IRC channel and addons-user-experience mailing list. I also got a better introduction to the projuct.
The exact bugs I must work on have been decided:
I will start of with bug 640775.
We decided which method of the possible methods to adopt to to fix it for now. I will soon submit a patch.
I have been going through documentation of the Add-on Manager API here. I was also going through the code trying to understand and relate things.
I'll very briefly write down some aspects I've understood so far. (Forgive the organization)
XPIProvider.jsm contains the XPIProvider object, AddonInstallWrapper, AddonInstall, Addon, AddonWrapper classes and many helper functions. It doesn't export any sysmbols. It registers itself as a provider of AddonManagerPrivate. AddonManagerPrivate which then has a reference to the provider can invoke methods of XPIProvider.
AddonManager which is wrapper over AddonManagerPrivate uses the provider(s) to access instances of Addon and AddonInstall classes. AddonManager is responsible for centrally managing all add-ons. addonManager.js implements amIWebInstaller interface. It makes use of AddonManager to install addons from a webpage.
Code that is responsible for handling the filesystem, windows registry, zipped files, parsing install.rdf is present in XPIProvider.jsm. This are much needed for add-on installation/uninstallation.
A typical add-on installation from a local file using AddonManager would take place like so:
- AddonManager's getInstallForFile is called -> XPIProvider's getInstallForFile is called -> AddonInstall's createInstall (static function) is called -> AddonInstall is created
- The created AddonInstall's initLocalInstall is called -> loadManifest is called -> install.rdf is parsed and the AddonInstall object gets an "addon" property which contains the parsed data
- An AddonInstall object is now obtained -> its install is called -> startInstall is called -> the add-on gets staged -> the add-on goes to its final location with the help of DirectoryInstallLocation class.
The code is quite big and I have been trying to find links between different classes, singletons, helper functions etc. to understand the flow.
