257304 - Use RDF instead of SOAP/WSDL for generic extension update service

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect, P2)

Description

[Ben Goodger (use ben at mozilla dot org for email)]

This web services business isn't going to scale, or have a hope of being shaken
out thoroughly for 1.0, there are problems with SSL etc. Thus we are falling
back to RDF to provide updates from update.mozilla.org. This bug tracks the
modifications to EM required to make this change.

Comment 1

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: initial work

not working yet

Comment 2

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: more...

contains other patches rolled up... in progress, not done yet.

Comment 3

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: more...

still not done...

Comment 4

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: patch

this one actually works but I need to do some more testing and clean some other
material out of this patch

Comment 5

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: patch

just needs cleaning up, now.

Comment 6

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: patch

still needs cleanup

Comment 7

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: patch

OK, this time's for real I think. 

Analysis coming.

Comment 8

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 157480 [details] [diff] [review]
patch

>-pref("app.extensions.version", "0.9");
>+pref("app.extensions.version", "0.10");

bump extensions compatibility

>-pref("update.app.enabled", true);               // Whether or not app updates are enabled
>-pref("update.app.url", "chrome://mozapps/locale/update/update.properties");
>-pref("update.app.updatesAvailable", false);
>-pref("update.app.updateVersion", "");
>-pref("update.app.updateDescription", "");
<snip>
reorg all pref names from update.app.* and update.extensions.* to app.update.*
and
extensions.update.* to better support extensions.{GUID}.update.* symmetry. 

>+pref("app.update.enabled", true);               // Whether or not app updates are enabled

New feature that shaver requested - disable all update checking, prevent the
user from 
invoking it. 

>+pref("app.update.autoUpdateEnabled", true);     // Whether or not background app updates 
>+                                                // are enabled

Old toggle for background update checking - new pref name for it that better
describes
the function. 

>+pref("extensions.update.enabled", true);

Extensions version of new shaver pref. 

>+pref("extensions.update.autoUpdateEnabled", true);

Old update pref, new name.

>+pref("extensions.dss.enabled", false);          // Dynamic Skin Switching                                               
>+pref("extensions.dss.switchPending", false);    // Non-dynamic switch pending after next
>+                                                // restart.

Turn off dynamic skin switching. 

>-pref("general.useragent.vendorSub",
>-#expand __APP_VERSION__
>-);
>+pref("general.useragent.vendorSub", "1.0 PR (NOT FINAL)");

Display version of user agent. Will be changed to "1.0 Preview Release" just
before 
actual release.

>Index: browser/app/profile/extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}/install.rdf.in

>-        <em:minVersion>0.9</em:minVersion>
>-        <em:maxVersion>0.9.0+</em:maxVersion>
>+        <em:minVersion>0.10</em:minVersion>
>+        <em:maxVersion>0.10</em:maxVersion>

Update for app.extensions.version bump. 

>Index: browser/base/content/browser.css

>-#statusbar-updates {
>+toolbarbutton[type="updates"] {

statusbar updates icon becomes a toolbar button, so style rules must change.

>Index: browser/base/content/browser.js
>-  
>-  // BiDi UI
>-  if (isBidiEnabled()) {
>-    document.getElementById("documentDirection-separator").hidden = false;
>-    document.getElementById("documentDirection-swap").hidden = false;
>-    document.getElementById("textfieldDirection-separator").hidden = false;
>-    document.getElementById("textfieldDirection-swap").hidden = false;
>-  }

Move BiDi initialization until after the window is shown to speed up window
open times and startup speed. Code with no immediate visual dependencies should
always go into delayedStartup()

>-  var updatePanel = document.getElementById("statusbar-updates");
>-  updatePanel.init();
>+  var updatePanel = document.getElementById("updates");
>+  try {
>+    updatePanel.init();
>+  }
>+  catch (e) { }

Now that update item is part of customizable toolbars, we need to use exception
handling in the case it isn't in the toolbars and so no XBL binding exists for
it.

>Index: browser/components/prefwindow/content/pref-advanced.xul
>-                  prefstring="update.app.enabled"/>
>+                  prefstring="app.update.autoUpdateEnabled"/>
>         <checkbox id="enableExtensionUpdate"
>                   label="&enableExtensionUpdate.label;" accesskey="&enableExtensionUpdate.accesskey;"
>-                  prefstring="update.extensions.enabled"/>      
>+                  prefstring="extensions.update.autoUpdateEnabled"/>      

reorg pref names in the preferences dialog.

>Index: browser/config/version.txt
>-0.9.1+
>+0.10

Update version number

>Index: browser/installer/unix/installer.cfg
>-VersionProduct            = 0.9.0+
>+VersionProduct            = 1.0 Preview Release
>Index: browser/installer/windows/installer.cfg
>-VersionProduct            = 0.9.0+
>+VersionProduct            = 1.0 Preview Release

New display version number

>Index: browser/locales/en-US/chrome/browser/browser.dtd
>+<!ENTITY updatesItem.title            "Updates">

String for customizable toolbar palette item for the updates button.

>Index: browser/locales/en-US/chrome/browser/pref/pref-advanced.dtd
>-<!ENTITY enableExtensionUpdate.label      "My Extensions">
>+<!ENTITY enableExtensionUpdate.label      "My Extensions and Themes">
> <!ENTITY enableExtensionUpdate.accesskey  "y">
>-<!ENTITY enableAutoInstall.label          "Automatically download and install updates to Extensions.">
>+<!ENTITY enableAutoInstall.label          "Automatically download and install updates to Extensions and Themes.">

use Themes in the terminology too, so it's clear.

>Index: browser/locales/en-US/chrome/global/brand.dtd
>-<!ENTITY  releaseURL            "http://www.mozilla.org/products/firefox/releases/0.9.html">
>+<!ENTITY  releaseURL            "http://www.mozilla.org/products/firefox/releases/0.10.html">

Rev version number. 

>Index: browser/locales/generic/install.rdf
>-        <em:minVersion>0.9</em:minVersion>
>-        <em:maxVersion>0.9.1+</em:maxVersion>
>+        <em:minVersion>0.10</em:minVersion>
>+        <em:maxVersion>0.10</em:maxVersion>

Rev version number.

>Index: browser/themes/winstripe/browser/browser.css
>+  display: none;
>+  display: -moz-box;
>+  display: -moz-box;
>+  display: -moz-box;
>+  display: none;
>+  display: -moz-box;
>+  display: none;
>+  display: -moz-box;
>+  display: none;
>+  display: -moz-box;

Status bar panels now show only when there is activity associated with them, as
before.

>Index: toolkit/locales/en-US/chrome/mozapps/extensions/extensions.dtd
>+<!ENTITY cmd.options.tooltip              "Set Options for the selected Extension">

Tooltip for the new "Options" button in Extension Manager.

>Index: toolkit/mozapps/extensions/content/extensions.js
>+const PREF_EXTENSIONS_DSS_ENABLED           = "extensions.dss.enabled";
>+const PREF_EXTENSIONS_DSS_SWITCHPENDING     = "extensions.dss.switchPending";

Dynamic Skin Switching enabling/disabling and state tracking prefs.

>+function setRestartMessage(aItem)

Utility function to set the message "Restart Firefox to use this theme" on the 
specified XUL element.

>+    var optionsButton = document.getElementById("optionsButton");
>+    optionsButton.hidden = true;

Hide the options button in Theme Manager mode

>+    var pref = Components.classes["@mozilla.org/preferences-service;1"]
>+                         .getService(Components.interfaces.nsIPrefBranch);
>+    if (!pref.getBoolPref(PREF_EXTENSIONS_DSS_ENABLED) && 
>+        pref.getBoolPref(PREF_EXTENSIONS_DSS_SWITCHPENDING)) {
>+      var lastSelectedSkin = pref.getCharPref(PREF_EM_LAST_SELECTED_SKIN);
>+      for (var i = 0; i < gExtensionsView.childNodes.length; ++i) {
>+        var item = gExtensionsView.childNodes[i];
>+        if (item.getAttribute("internalName") == lastSelectedSkin) {
>+          setRestartMessage(item);
>+          break;
>+        }
>+      }
>+    }

If the user opened the Theme Manager before and switched themes, then closed
the window, when they reopen it this session be sure to set the Restart message
on the element for the theme they switched to.

>-      item.init(url, " ", "", "", displayName, -1, url, iconURL, "", "", type);
>+      item.init(url, " ", "", "", displayName, -1, url, iconURL, "", type);

No more serviceURL property on nsIUpdateItem objects

>+      if (pref.getBoolPref(PREF_EXTENSIONS_DSS_ENABLED)) {
>+        cr.selectSkin(gCurrentTheme, true);
>+        cr.refreshSkins();
>+      }

Only switch skins dynamically if a pref is set. 

>+      else {
>+        // Theme change will happen on next startup, this flag tells
>+        // the Theme Manager that it needs to show "This theme will
>+        // be selected after a restart" text in the selected theme
>+        // item.
>+        pref.setBoolPref(PREF_EXTENSIONS_DSS_SWITCHPENDING, true);
>+        // Update the view
>+        setRestartMessage(aSelectedItem);

Otherwise say that there's a pending switch and update the UI element. 

>Index: toolkit/mozapps/extensions/content/extensions.xml
>-          <xul:label class="extension-item-creator" xbl:inherits="value=creator" crop="right"/>
>-        </xul:vbox>
>-        <xul:vbox pack="start">
>-          <xul:image class="extension-button" anonid="options-button" 

Remove tiny non-accessible buttons from each extension item in extension view, 
and author name to trim down the view a bit. 

>-    <implementation>
>-      <field name="eventPrefix">"extension-"</field>
>-    </implementation>
> ...

Remove code that handles the tiny non-accessible button actions.

>Index: toolkit/mozapps/extensions/content/extensions.xul
>+        <button id="optionsButton"
>+                label="&cmd.options.label;" accesskey="&cmd.options.accesskey;" tooltiptext="&cmd.options.tooltip;"
>+                command="cmd_options"/>

Add Options button...

>Index: toolkit/mozapps/extensions/public/nsIExtensionManager.idl
>-  const unsigned long UPDATE_MODE_VERSION = 0x01;
>-  const unsigned long UPDATE_MODE_NORMAL  = 0x02;
>   void update([array, size_is(aItemCount)] in nsIUpdateItem aItems,
>                in unsigned long aItemCount, 
>-               in unsigned long aUpdateMode,
>-               in boolean aWriteVersionUpdates);
>+               in unsigned long aVersionUpdateOnly);

Handle normal vs. version updates through a boolean, flags are dangerous
as there is no meaningful combination of the two modes. 

>   void checkForUpdates([array, size_is(aItemCount)] in nsIUpdateItem aItems,
>                        in unsigned long aItemCount, 
>-                       in unsigned long aUpdateMode,
>-                       in boolean aWriteVersionUpdates);
>+                       in unsigned long aUpdateMode);

Error in patch - this should read:

  in unsigned boolean aVersionUpdateOnly)

as the last parameter. Fixed in tree. Simplify API as a result of new boolean
logic to determine version only vs. normal. 

>Index: toolkit/mozapps/extensions/src/nsExtensionManager.js.in
>-const PREF_UPDATE_COUNT               = "update.extensions.count";
>-const PREF_UPDATE_EXT_WSDL_URI        = "update.extensions.wsdl";
>+const PREF_UPDATE_COUNT               = "extensions.update.count";
>+const PREF_UPDATE_DEFAULT_URL         = "extensions.update.url";
> const PREF_EM_WASINSAFEMODE           = "extensions.wasInSafeMode";

reorg pref names. 

>+const ERROR_EXTENSION_IS_THEME        = -3;

pref for when a theme is updated using broken XPInstall control flows that
install .jars as .XPI extensions. 

>+var gPref           = null;
>+var gRDF            = null;
>+var gOS             = null;
>+var gVersionChecker = null;

Use single globals for services to better maintain object lifetimes. 

>+function getVersionChecker()
>+{
>+  if (!gVersionChecker) {
>+    gVersionChecker = Components.classes["@mozilla.org/updates/version-checker;1"]
>+                                .getService(Components.interfaces.nsIVersionChecker);
>+  }
>+  return gVersionChecker;
>+}

Get VC on demand. 

> function getItemRoots(aItemType)
> {    
>   var roots = [];
>-  if (aItemType & nsIUpdateItem.TYPE_ADDON)
>+  if (aItemType == nsIUpdateItem.TYPE_ADDON)
>     roots = roots.concat([getItemRoot(nsIUpdateItem.TYPE_EXTENSION), 
>                           getItemRoot(nsIUpdateItem.TYPE_THEME)]);

Part of bitwise logic sanity check. If all addon bits are set for types we
currently
support (extensions and themes, then use roots for each of those types. 

>-  var prefix = getItemPrefix(aItemType);
>-  if (prefix) 
>-    val = aURI.substr(prefix.length, aURI.length);
>-  else // aItemType = nsIUpdateItem.TYPE_ADDON
>+  if (aItemType == nsIUpdateItem.TYPE_ADDON)
>     val = stripPrefix(aURI, getItemType(aURI));
>+  else {
>+    var prefix = getItemPrefix(aItemType);
>+    if (prefix) 
>+      val = aURI.substr(prefix.length, aURI.length);
>+  }

Bitwise logic cleanup. 

>+function stripPropertyPrefix(aProperty, aPrefix)
>+{
>+  return aProperty.substr(aPrefix.length, aProperty.length);
>+}
>+

Strip EM_NS/CHROME_NS/etc off the front of a property resource URI. 

>-    var rdf = Components.classes["@mozilla.org/rdf/rdf-service;1"]
>-                        .getService(Components.interfaces.nsIRDFService);
>-    
>     var rdfc = Components.classes["@mozilla.org/rdf/container;1"]
>                          .createInstance(Components.interfaces.nsIRDFContainer);
>-    rdfc.Init(aDS, rdf.GetResource("urn:mozilla:skin:root"));
>+    rdfc.Init(aDS, gRDF.GetResource("urn:mozilla:skin:root"));
>     
>     var elts = rdfc.GetElements();
>-    var nameArc = rdf.GetResource(CHROME_NS("displayName"));
>+    var nameArc = gRDF.GetResource(CHROME_NS("displayName"));

Use new global variables. 

>+
>+function ArrayEnumerator(aItems)
>+{
>...

nsISimpleEnumerator implementation...

>-      dump("******* EEEEEE = " + e + "\n");
>+      dump("******* Failed to remove extension uninstall log, with exception = " + e + "\n");
>-      dump("******* EEEEEE = " + e + "\n");
>+      dump("******* Failed to remove theme uninstall log, with exception = " + e + "\n");

Better error reporting...

>-  var os = Components.classes["@mozilla.org/observer-service;1"]
>-                     .getService(Components.interfaces.nsIObserverService);
>-  os.addObserver(this, "profile-after-change", false);
>+  gPref = Components.classes["@mozilla.org/preferences-service;1"]
>+                    .getService(Components.interfaces.nsIPrefBranch);
>+  gRDF = Components.classes["@mozilla.org/rdf/rdf-service;1"]
>+                   .getService(Components.interfaces.nsIRDFService);
>+  gOS = Components.classes["@mozilla.org/observer-service;1"]
>+                  .getService(Components.interfaces.nsIObserverService);
>+
>+  gOS.addObserver(this, "xpcom-shutdown", false);

Get global services, and add a shutdown observer to free them...

>+    case "xpcom-shutdown":
>+      gOS.removeObserver(this, "xpcom-shutdown");    
>+
>+      // Release strongly held services.
>+      gPref           = null;
>+      gRDF            = null;
>+      gOS             = null;
>+      gVersionChecker = null;
>+      break;

... self explanatory ...

>+    // Clear the DSS settings since once CR loads, it'll perform the
>+    // skin switch.
>+    if (!gPref.getBoolPref(PREF_EM_DSS_ENABLED) && 
>+        gPref.getBoolPref(PREF_EM_DSS_SWITCHPENDING))
>+      gPref.setBoolPref(PREF_EM_DSS_SWITCHPENDING, false);
>+

When a skin is switched and DSS is disabled, a pending pref is set so that when
the 
THeme manager is closed and reopened, the "restart" message persists on the 
pending-switched-to theme. This pending pref needs to be cleared the next time
the
EM is started, since the CR will do the skin switch when it inits. 

>     var extensionID = this.installExtensionInternal(xpiFile, tempManifest, installProfile);
>-    if (extensionID) {
>+    switch (extensionID) {
>+    case ERROR_EXTENSION_IS_THEME:
>+      this.installTheme(aXPIFile, aFlags);
>+      break;

installExtensionInternal deemed this XPI to actually be a theme jar, so call
that function 
instead. 

>+    case ERROR_INVALID_VERSION:
>+    case ERROR_PHONED_HOME:
>+      break;

We shouldn't stage or update the component manifest yet if this was an invalid
item, or
there is a phone home op in progress. In the latter case that will be done if
home 
"picks up".

>+    // XXXben - this is a hack until we properly fix xpinstall to be able to install
>+    //          different chrome types from trusted script. At the moment, when we
>+    //          call initManagerFromChrome, we can only install extensions, since
>+    //          the code path that installs themes is not utilized. To minimize the
>+    //          level of changes happening at the lower level in xpinstall at this
>+    //          point I am inserting this hack which checks for a theme-only property
>+    //          in the install manifest.
>+    var manifestRoot = gRDF.GetResource("urn:mozilla:install-manifest");
>+    var internalName = gRDF.GetResource(EM_NS("internalName"));
>+    if (stringData(ds.GetTarget(manifestRoot, internalName, true)) != "--")
>+      return ERROR_EXTENSION_IS_THEME;
>+    

Read the comment. 

>-        var os = Components.classes["@mozilla.org/observer-service;1"]
>-                           .getService(Components.interfaces.nsIObserverService);
>-        os.addObserver(this, "Version:Extension:Started", false);
>-        os.addObserver(this, "Version:Extension:Item-Ended", false);
>-        os.addObserver(this, "Version:Extension:Item-Error", false);
>-        os.addObserver(this, "Version:Extension:Ended", false);
>+        gOS.addObserver(this, "Update:Extension:Started", false);
>+        gOS.addObserver(this, "Update:Extension:Item-Ended", false);
>+        gOS.addObserver(this, "Update:Extension:Item-Error", false);
>+        gOS.addObserver(this, "Update:Extension:Ended", false);

Simplification of Version-Only checking means that we can reuse extension
update 
notification messages here. 

> 
>-        this._em.update([item], 1, nsIExtensionManager.UPDATE_MODE_VERSION, false);
>+        this._em.update([item], 1, true);

nsIExtensionManager API changed. 

>-      case "Version:Extension:Started":
>+      case "Update:Extension:Started":

message simplification...

>-      newItem.init(id, version, targetAppInfo.minAppVersion, 
>-                   targetAppInfo.maxAppVersion,
>-                   name, -1, "", "", "", "", aType);
>+      newItem.init(id, version, targetAppInfo.minVersion, 
>+                   targetAppInfo.maxVersion,
>+                   name, -1, "", "", "", aType);

No more serviceURL on nsIUpdateItem...

>-    updater.checkForUpdates(aItems, aItems.length, aUpdateMode, 
>-                            aWriteVersionUpdates);
>+    updater.checkForUpdates(aItems, aItems.length, aVersionUpdateOnly);

nsIExtensionItemUpdater simplification - boolean logic Version Only vs. Normal. 

>-  this._os = Components.classes["@mozilla.org/observer-service;1"]
>-                       .getService(Components.interfaces.nsIObserverService);
>-
>-  this._versionChecker = Components.classes["@mozilla.org/updates/version-checker;1"]
>-                                   .getService(Components.interfaces.nsIVersionChecker);

These are now globals

>-  this._goat = Math.round(Math.random() * 100000);

Who knows.

>+  getVersionChecker();
> }

>-  _getMessage: function nsExtensionItemUpdater_getMessage(aMessage)
>-  {
>-    return this._prefix + ":Extension:" + aMessage;
>-  }

Observer Messages are now simplified, no need for special generation of
Version-only
vs. Extension Update. 

>+  //
>+  // When we check for updates to an item, there are two pieces of information
>+  // that are returned - 1) info about the newest available version, if any,
>+  // and 2) info about the currently installed version. The latter is provided
>+  // primarily to inform the client of changes to the application compatibility 
>+  // metadata for the current item. Depending on the situation, either 2 or 
>+  // 1&2 may be what is required.
>+  //
>+  // Callers:
>+  //  1 - nsUpdateService.js, user event
>+  //      User clicked on the update icon to invoke an update check, 
>+  //      user clicked on an Extension/Theme and clicked "Update". In this
>+  //      case we want to update compatibility metadata about the installed
>+  //      version, and look for newer versions to offer. 
>+  //  2 - nsUpdateService.js, background event
>+  //      Timer fired, background update is being performed. In this case
>+  //      we also want to update compatibility metadata and look for newer
>+  //      versions.
>+  //  3 - Mismatch
>+  //      User upgraded to a newer version of the app, update compatibility
>+  //      metadata and look for newer versions.
>+  //  4 - Install Phone Home
>+  //      User installed an item that was deemed incompatible based only
>+  //      on the information provided in the item's install.rdf manifest, 
>+  //      we look ONLY for compatibility updates in this case to determine
>+  //      whether or not the item can be installed.
>+  //  

Better documentation describing usage and entry points...

>-    this._prefix = (aUpdateMode == nsIExtensionManager.UPDATE_MODE_NORMAL) ? "Update" : "Version";
>-    this._os.notifyObservers(null, this._getMessage("Started"), "");
>-    this._updateMode = aUpdateMode;
>+    gOS.notifyObservers(null, "Update:Extension:Started", "");
>+    this._versionUpdateOnly = aVersionUpdateOnly;

Simplified messages and boolean logic...

>-      this._os.notifyObservers(e, this._getMessage("Item-Started"), "");
>-      if (e.updateRDF)
>-        (new nsRDFItemUpdater(this)).checkForUpdates(e, this._updateMode);
>-      else {
>-        // Invoke a custom web service proxy, if the extension specifies one.
>-        if (e.updateService)
>-          this._checkForUpdatesAtServiceURI(e.updateService);
>-        else
>-          defaultRequest = true;
>-      }

Die WS, Die. 

>+      gOS.notifyObservers(e, "Update:Extension:Item-Started", "");
>+      (new nsRDFItemUpdater(this)).checkForUpdates(e, aVersionUpdateOnly);

Use an RDFItemUpdater instead. 

>-    if (defaultRequest) {
>-      var wsdlURI = pref.getComplexValue(PREF_UPDATE_EXT_WSDL_URI,
>-                                         Components.interfaces.nsIPrefLocalizedString).data;
>-      this._checkForUpdatesAtServiceURI(wsdlURI);
> ...
Die WS, Die.

>     dsURI = dsURI.replace(/%ITEM_ID%/g, aItem.id);
>     dsURI = dsURI.replace(/%ITEM_VERSION%/g, aItem.version);
>+    dsURI = dsURI.replace(/%ITEM_MAXAPPVERSION%/g, aItem.maxAppVersion);
>     dsURI = dsURI.replace(/%APP_ID%/g, this._updater._appID);
>     dsURI = dsURI.replace(/%APP_VERSION%/g, this._updater._appVersion);
>-    var ds = this._rdf.GetDataSource(dsURI);
>+    dsURI = dsURI.replace(/%REQ_VERSION%/g, 1);
>+    
>+    // escape() does not properly encode + symbols in any embedded FVF strings.
>+    dsURI = dsURI.replace(/\+/g, "%2B");
>+
>+    var ds = gRDF.GetDataSource(dsURI);

Format the GET query string for the VersionCheck.php datasource. 

>-      sink.addXMLSinkObserver(new nsExtensionUpdateXMLRDFDSObserver(this, aItem));
>+      sink.addXMLSinkObserver(this);

Let's not create silly dummy object proxies. 

>-    //      <Seq>
>-    //        <li resource="urn:mozilla:extension:{GUID}:4.9"/>
>-    //        <li resource="urn:mozilla:extension:{GUID}:5.0"/>
>-    //      </Seq>
>+    //      <RDF:Seq>
>+    //        <RDF:li resource="urn:mozilla:extension:{GUID}:4.9"/>
>+    //        <RDF:li resource="urn:mozilla:extension:{GUID}:5.0"/>
>+    //      </RDF:Seq>

Comments should contain valid XML. 

>+    // Firefox 1.0PR+ update.rdf format
>+    if (!this._versionUpdateOnly) {

Unify Version Update check and Update Check into one remote request. When
in normal mode, parse the response to get info for the newest version, then
in all modes parse the response to get info for the current version. 

>-function nsExtensionUpdateXMLRDFDSObserver(aUpdater, aItem)

Kill this worthless proxy. 

>-        if (getItemType(e.Value) != -1 && !this.isCompatible(this, e)) {
>-          var itemType = getItemType(e.Value);
>-          var id = stripPrefix(e.Value, itemType);
>-          var item = Components.classes["@mozilla.org/updates/item;1"]
>-                               .createInstance(Components.interfaces.nsIUpdateItem);
>-          item.init(id, this._getItemProperty(e, "version"), "", "",
>-                    this._getItemProperty(e, "name"),
>-                    -1, "", "", this._getItemProperty(e, "updateURL"), "", 
>-                    itemType);
>-          items.push(item);
>-        }
>+        var itemType = getItemType(e.Value);
>+        if (itemType != -1 && !this.isCompatible(this, e))
>+          items.push(this.getItemForID(stripPrefix(e.Value, itemType)));

Rather than generate a new item each time, use a utility function that we
already had
instead, to reduce code complexity. 

>+    if (aItemID)
>+      items.push(this.getItemForID(aItemID));

Reuse utility function...

>-          if (getItemType(e.Value) != -1) {
>-            var id = stripPrefix(e.Value, aType);
>-            var item = Components.classes["@mozilla.org/updates/item;1"]
>-                                .createInstance(Components.interfaces.nsIUpdateItem);
>-            item.init(id, this.getItemProperty(id, "version"), "", "",
>-                      this.getItemProperty(id, "name"),
>-                      -1, "", "", this.getItemProperty(id, "updateURL"), 
>-                      "", getItemType(roots[i]));
>-            items.push(item);
>-          }
>+          var itemType = getItemType(e.Value);
>+          if (itemType != -1)
>+            items.push(this.getItemForID(stripPrefix(e.Value, itemType)));

Reuse utility function...

>-              targetAppInfo.minVersion,
>-              targetAppInfo.maxVersion,
>+              targetAppInfo ? targetAppInfo.minVersion : "",
>+              targetAppInfo ? targetAppInfo.maxVersion : "",

We don't always have targetAppInfo. 

>-              "", getItemType(r.Value));
>+              getItemType(r.Value));

no more serviceURL on nsIUpdateItem

>+    else if (aProperty.EqualsNode(this._emR("name")) || 
>+             aProperty.EqualsNode(this._emR("description")) || 
>+             aProperty.EqualsNode(this._emR("creator")) || 
>+             aProperty.EqualsNode(this._emR("homepageURL"))) {
>+      // These are localizable properties that a language pack supplied by the 
>+      // Extension may override.          
>+      var prefName = PREF_EM_EXTENSION_FORMAT.replace(/%UUID%/, 
>+                                                      stripPrefix(aSource.Value, 
>+                                                                  nsIUpdateItem.TYPE_EXTENSION)) + 
>+                      stripPropertyPrefix(aProperty.Value, EM_NS_PREFIX);
>+      try {
>+        var value = gPref.getComplexValue(prefName, 
>+                                          Components.interfaces.nsIPrefLocalizedString);
>+        if (value.data) 
>+          return this._emL(value.data);
>+      }
>+      catch (e) {
>+      }
>+    }

Support localizable Extension Manager metadata properties, in the form:

pref("extensions.{GUID}.[name|description|creator|homepageURL]",
"chrome://foo/locale/foo.properties");

>+    if (aProperty.EqualsNode(this._emR("name")) ||
>+        aProperty.EqualsNode(this._emR("contributor"))) {

Ditto, but for the GetTargets case, and for .contributor, which can take
multiple values, e.g.:

extensions.{GUID}.contributor.1
extensions.{GUID}.contributor.2
extensions.{GUID}.contributor.3
...

>Index: toolkit/mozapps/update/content/update.js
>-const PREF_APP_ID                           = "app.id";
>-const PREF_UPDATE_EXTENSIONS_ENABLED        = "update.extensions.enabled";
>-const PREF_UPDATE_APP_ENABLED               = "update.app.enabled";
>-const PREF_UPDATE_APP_UPDATESAVAILABLE      = "update.app.updatesAvailable";
>...
pref name reorg and whitespace

... whole bunch of code in init() function for wizard that does away with the
separate Version Update check page in the wizard sequence. 

>-    gVersionPage.uninit();

kill version page

>-    if (pref.prefHasUserValue(PREF_UPDATE_APP_UPDATEVERSION))
>-      pref.clearUserPref(PREF_UPDATE_APP_UPDATEVERSION);
>-    if (pref.prefHasUserValue(PREF_UPDATE_APP_UPDATEURL))
>-      pref.clearUserPref(PREF_UPDATE_APP_UPDATEDESCRIPTION);
>-    if (pref.prefHasUserValue(PREF_UPDATE_APP_UPDATEURL)) 
>-      pref.clearUserPref(PREF_UPDATE_APP_UPDATEURL);

kill obsolete prefs

>-var gVersionPage = {
>-  _completeCount: 0,
>-  _messages: ["Version:Extension:Started", 
>-              "Version:Extension:Ended", 
>-              "Version:Extension:Item-Started", 
>-              "Version:Extension:Item-Ended",
>-              "Version:Extension:Item-Error"],

Version updates handled at the same time as regular updates now, kill
the entire version page object. 

>-      if (gUpdateWizard.itemsToUpdate.length > 0 || updates.appUpdatesAvailable)
>+      if ((gUpdateTypes & nsIUpdateItem.TYPE_ADDON && gUpdateWizard.itemsToUpdate.length > 0) || 
>+          (gUpdateTypes & nsIUpdateItem.TYPE_APP && updates.appUpdatesAvailable))

Don't show the "updates found page" unless we were checking for updates of that
type, AND
there were updates of that type available... prevents "blank wizard page" when
no updates
found bug. 

>     gUpdateWizard.setButtonLabels(null, true, 
>                                   "installButtonText", false, 
>-                                  null, true);
>+                                  null, false);

Make sure cancel button is available.

>+    if (!this._initialized) {
>+      this._initialized = true;

Initialize a bunch of stuff ONCE...

>+    }
>+        
>     var kids = updates._getRadioChildren();
>     for (var i = 0; i < kids.length; ++i) {
>       if (kids[i].collapsed == false) {
>         updates.selectedIndex = i;
>         break;
>       }
>     }
>   },

And then execute this block *every* time the page is shown (i.e. when the user
clicks
Next then Back) to make sure that there's an item selected in the list.

>         gUpdateWizard.setButtonLabels(null, true, 
>                                       "nextButtonText", true, 
>-                                      null, true);
>+                                      null, false);

Cancel should (almost) always be available...

>+    while (optionalItemsList.hasChildNodes())
>+      optionalItemsList.removeChild(optionalItemsList.firstChild);
>+

Clear the Optional Components list each time we view this page so that repeated
back/
fwd operations doesn't make it grow stupidly. 

>Index: toolkit/mozapps/update/content/update.xul
>-<wizard id="migrationWizard"
>+<wizard id="updateWizard"

Oops.

>+        windowtype="Update:Wizard"

Window type for Window Mediator...

>-  <wizardpage id="version" pageid="version" next="checking"

Remove version check page.

>Index: toolkit/mozapps/update/content/updates.xml
>-  <binding id="updateStatusbarNotification" extends="chrome://global/content/bindings/general.xml#statusbarpanel">
>+  <binding id="updateStatusbarNotification" 
>+           extends="chrome://global/content/bindings/toolbarbutton.xml#toolbarbutton-image">

Update icon is now a toolbar button

>-    <content>
>-      <xul:hbox flex="1" class="updateIndicator" xbl:inherits="updateCount"
>-                anonid="infoPanel">
>-        <xul:image xbl:inherits="severity" class="updateIcon"/>
>-        <xul:label xbl:inherits="value=updateCount" flex="1" crop="right"/>
>-      </xul:hbox>
>-    </content>

... and no longer has all this complex content

>-      <handler event="dblclick">
>+      <handler event="command">
>       <![CDATA[
>-        if (event.button == 0)
>-          this.showUpdates();
>+        this.showUpdates();

... and as a result event fires on click, not dblclick.

>Index: toolkit/mozapps/update/public/nsIUpdateService.idl
>-  readonly attribute wstring  updateService;

remove custom serviceURL.

>-  const unsigned short TYPE_ANY         = 0x01;
>-  const unsigned short TYPE_APP         = 0x02;
>-  const unsigned short TYPE_ADDON       = 0x04;
>-
>-  const unsigned short TYPE_EXTENSION   = 0x08;
>-  const unsigned short TYPE_THEME       = 0x10;
>-  const unsigned short TYPE_LOCALE      = 0x20;
>+  
>+  const unsigned short TYPE_APP         = 0x01;
>+  const unsigned short TYPE_EXTENSION   = 0x02;
>+  const unsigned short TYPE_THEME       = 0x04;
>+  const unsigned short TYPE_LOCALE      = 0x08;
>+  const unsigned short TYPE_ADDON       = TYPE_EXTENSION + TYPE_THEME + TYPE_LOCALE;
>+  const unsigned short TYPE_ANY         = TYPE_APP + TYPE_ADDON;

Fix bitwise logic here to make sense for once. 

>   void init(in string aID, in string aVersion, in string aMinAppVersion, 
>             in string aMaxAppVersion, in wstring aName, 
>             in long aRow, in wstring aUpdateURL, in wstring aIconURL, 
>-            in wstring aUpdateRDF, in wstring aUpdateService,
>-            in long aType);
>+            in wstring aUpdateRDF, in long aType);

remove service URL

>Index: toolkit/mozapps/update/src/nsUpdateService.js.in
>-const PREF_UPDATE_APP_ENABLED               = "update.app.enabled";
>-const PREF_UPDATE_APP_URI                   = "update.app.url";
>-const PREF_UPDATE_APP_UPDATESAVAILABLE      = "update.app.updatesAvailable";
>...

reorg pref names

> function getOSKey()
> {
> #ifdef XP_UNIX
> #ifdef XP_MACOSX
>-  return "mac";
>+  return "macosx";
> #else
>-  return "lin";
>+  return "linux";
> #endif
> #else
>-  return "win";
>+  return "windows";
> #endif

Rename platforms... shaver can swoop in and replace this with sexy
substitution.

>+  pbi.addObserver(PREF_UPDATE_APP_AUTOUPDATEENABLED, this, false);
>   pbi.addObserver(PREF_UPDATE_APP_ENABLED, this, false);
>+  pbi.addObserver(PREF_UPDATE_EXTENSIONS_AUTOUPDATEENABLED, this, false);

New prefs to support disabling of background update checking now that the old
pref names
have been co-opted for new shaver feature.

>-  _appUpdatesEnabled: true,
>-  _extUpdatesEnabled: true,
>+  _appAutoUpdateEnabled: true,
>+  _extAutoUpdateEnabled: true,

Better names to reflect function.

>-    this._appUpdatesEnabled = gPref.getBoolPref(PREF_UPDATE_APP_ENABLED);
>-    this._extUpdatesEnabled = gPref.getBoolPref(PREF_UPDATE_EXTENSIONS_ENABLED);
>-    if (!this._appUpdatesEnabled && !this._extUpdatesEnabled)
>+    this._appAutoUpdateEnabled = gPref.getBoolPref(PREF_UPDATE_APP_AUTOUPDATEENABLED);
>+    this._extAutoUpdateEnabled = gPref.getBoolPref(PREF_UPDATE_EXTENSIONS_AUTOUPDATEENABLED);
>+    if (!this._appAutoUpdateEnabled && !this._extAutoUpdateEnabled)

... update pref names ...

>+  _getAllowedTypes: function nsUpdateService__getAllowedTypes(aRequestedTypes)
>+  {
>+    // Figure out what types we're allowed to update. These options
>+    // differ from PREF_UPDATE_*_AUTOUPDATEENABLED since they effectively
>+    // shut down the update UI if the administrator/distributor has configured
>+    // a build to have disallowed these types of update.
>+    var extUpdateEnabled = gPref.getBoolPref(PREF_UPDATE_EXTENSIONS_ENABLED);
>+    var appUpdateEnabled = gPref.getBoolPref(PREF_UPDATE_APP_ENABLED);
>+    
>+    var types = 0;
>+    if (extUpdateEnabled) {
>+      if (aRequestedTypes & nsIUpdateItem.TYPE_EXTENSION)
>+        types |= nsIUpdateItem.TYPE_EXTENSION;
>+      if (aRequestedTypes & nsIUpdateItem.TYPE_THEME)
>+        types |= nsIUpdateItem.TYPE_THEME;
>+    }
>+    if (appUpdateEnabled && 
>+        (aRequestedTypes & nsIUpdateItem.TYPE_APP))
>+      types |= nsIUpdateItem.TYPE_APP;
>+
>+    return types;
>+  },

implement shaver feature.

>   checkForUpdates: function nsUpdateService_checkForUpdates (aItems, aItemCount, aUpdateTypes, aSourceEvent, aParentWindow)
>   {
>+    var types = this._getAllowedTypes(aUpdateTypes);
>+    
>+    // Nothing to update
>+    if (!types) {
>+      var sbs = Components.classes["@mozilla.org/intl/stringbundle;1"]
>+                          .getService(Components.interfaces.nsIStringBundleService);
>+      var bundle = sbs.createBundle("chrome://mozapps/locale/update/update.properties");
>+      var ps = Components.classes["@mozilla.org/embedcomp/prompt-service;1"]
>+                         .getService(Components.interfaces.nsIPromptService);
>+      ps.alert(aParentWindow, 
>+               bundle.GetStringFromName("updatesdisabledtitle"), 
>+               bundle.GetStringFromName("updatesdisabledmessage"));
>+      return;
>+    }

See if we're allowed to update anything, and if not, show an error msg.

>+      
>+      var wm = Components.classes["@mozilla.org/appshell/window-mediator;1"]
>+                         .getService(Components.interfaces.nsIWindowMediator);
>+      var wizard = wm.getMostRecentWindow("Update:Wizard");
>+      if (wizard)
>+        wizard.focus();

Focus an existing Update Wizard if available, rather than creating many.

>+      this.checkForUpdatesInternal([], 0, types, aSourceEvent);

Use sanitized types flag now post-shaverfication.

>   _canUpdate: function (aPreference, aSourceEvent, aUpdateTypes)
>   {
>...
>+    // Always can update if the autoupdate preference is set, otherwise, 
>+    // allow updates only when not in backround update mode, i.e. when the user
>+    // explicitly asked. 
>+    return aPreference ? true 
>+                       : aSourceEvent != nsIUpdateService.SOURCE_EVENT_BACKGROUND;
>   },

Simplify logic. 

>+    var types = this._getAllowedTypes(aUpdateTypes);

Shaverfication.

>-    this._updateObserver = new nsUpdateObserver(aUpdateTypes, aSourceEvent, this);
>+    this._updateObserver = new nsUpdateObserver(types, aSourceEvent, this);

Use sanitized types flag post-shaverfication.

>-    if ((aUpdateTypes & nsIUpdateItem.TYPE_ANY) || (aUpdateTypes & nsIUpdateItem.TYPE_APP)) {
>-      if (this._canUpdate(this._appUpdatesEnabled, aSourceEvent, aUpdateTypes)) {
>+    if (types & nsIUpdateItem.TYPE_APP) {
>+      if (this._canUpdate(this._appAutoUpdateEnabled, aSourceEvent, types)) {

Proper bitwise logic.

>-    if (!(aUpdateTypes & nsIUpdateItem.TYPE_APP)) { // TYPE_EXTENSION, TYPE_ANY, etc.
>-      if (this._canUpdate(this._extUpdatesEnabled, aSourceEvent, aUpdateTypes)) {
>+    if (types & nsIUpdateItem.TYPE_ADDON) { // TYPE_EXTENSION, TYPE_ANY, etc.
>+      if (this._canUpdate(this._extAutoUpdateEnabled, aSourceEvent, types)) {

Proper bitwise logic.

>-        em.update(aItems, aItems.length, 
>-                  nsIExtensionManager.UPDATE_MODE_NORMAL, false);
>+        em.update(aItems, aItems.length, false);

simplified nsIExtensionManager API to deal with version updates.

>     if (aSourceEvent == nsIUpdateService.SOURCE_EVENT_BACKGROUND && 
>-        (this._appUpdatesEnabled || this._extUpdatesEnabled)) {
>-      if (aUpdateTypes & nsIUpdateItem.TYPE_ADDON)
>+        (this._appAutoUpdateEnabled || this._extAutoUpdateEnabled)) {
>+      if (types & nsIUpdateItem.TYPE_ADDON)
>         gPref.setIntPref(PREF_UPDATE_EXTENSIONS_LASTUPDATEDATE, this._nowInMilliseconds / 1000);
>-      if (aUpdateTypes & nsIUpdateItem.TYPE_APP)
>+      if (types & nsIUpdateItem.TYPE_APP)
>         gPref.setIntPref(PREF_UPDATE_APP_LASTUPDATEDATE, this._nowInMilliseconds / 1000);

sanitized flags and updated pref names

>     case "nsPref:changed":
>       var needsNotification = false;
>       switch (aData) {
>+      case PREF_UPDATE_APP_AUTOUPDATEENABLED:

Check for changes to these prefs now too...

>-    if (gPref.prefHasUserValue(PREF_UPDATE_APP_UPDATEVERSION))
>-      gPref.clearUserPref(PREF_UPDATE_APP_UPDATEVERSION);
>-    if (gPref.prefHasUserValue(PREF_UPDATE_APP_UPDATEURL))
>-      gPref.clearUserPref(PREF_UPDATE_APP_UPDATEDESCRIPTION);
>-    if (gPref.prefHasUserValue(PREF_UPDATE_APP_UPDATEURL)) 
>-      gPref.clearUserPref(PREF_UPDATE_APP_UPDATEURL);

Remove obsolete pref references...

>   get _doneUpdating()
>   {
>-    var appUpdatesEnabled = this._service._appUpdatesEnabled;
>-    var extUpdatesEnabled = this._service._extUpdatesEnabled;
>-  
>+    var notBackground = this._sourceEvent != nsIUpdateService.SOURCE_EVENT_BACKGROUND;
>+    var canUpdateApp = this._service._appAutoUpdateEnabled || 
>+                        (notBackground ? gPref.getBoolPref(PREF_UPDATE_APP_ENABLED) 
>+                                       : false);
>+    var canUpdateExt = this._service._extAutoUpdateEnabled || 
>+                        (notBackground ? gPref.getBoolPref(PREF_UPDATE_EXTENSIONS_ENABLED) 
>+                                       : false);
>+   

Shaverfication. 

>+      var fileArc = gRDF.GetResource(APP_NS("file"));
>+      var platform = getOSKey();
>       while (elements.hasMoreElements()) {
>         var element = elements.getNext().QueryInterface(Components.interfaces.nsIRDFResource);
>         var info = new nsAppUpdateInfoItem();
>         info.name          = this._getPropertyFromResource(aDataSource, element, "name");
>         info.internalName  = this._getPropertyFromResource(aDataSource, element, "internalName");
>-        info.URL           = this._getPropertyFromResource(aDataSource, element, getOSKey() + "URL");
>+        
>+        // Each Component has a set of app:file arcs out, which reference resources with two 
>+        // properties: app:platform and app:URL. If we find a resource whose app:platform value
>+        // matches the platform we're running on, we use the app:URL property on that resource
>+        // as the XPI URL, otherwise we use the default app:URL property on the Component
>+        // resource. (It must be a cross-platform piece, e.g. a language pack)
>+        // XXXben - what to do when platform not supported? We need some way to abort 
>+        //          and tell the app that this update is not available. 
>+        var files = aDataSource.GetTargets(element, fileArc, true);
>+        while (files.hasMoreElements()) {
>+          var file = files.getNext().QueryInterface(Components.interfaces.nsIRDFResource);
>+          if (platform == this._getPropertyFromResource(aDataSource, file, "platform")) {
>+            info.URL = this._getPropertyFromResource(aDataSource, file, "URL");
>+            break;
>+          }
>+        }

Update update rdf format for the app to take platform names out of the lexicon. 

>Index: toolkit/mozapps/update/src/update.rdf

Sample of the new update rdf format.

>Index: toolkit/themes/winstripe/mozapps/extensions/extensions.css
> #uninstallButton {
>   margin: 0px;
>-  -moz-user-focus: ignore;

Command bar buttons should be focusable.

>+#optionsButton {

Add options button.

>Index: toolkit/themes/winstripe/mozapps/update/update.css
>-.updateIndicator[updateCount="0"] {
>-  visibility: hidden;
>+toolbarbutton[type="updates"] {

Various styles for the update icon on the toolbar.

>Index: toolkit/xre/nsAppRunner.cpp
>Index: toolkit/xre/nsXREDirProvider.cpp

See bug 252543...

>Index: browser/base/content/browser.xul
> #ifdef XP_MACOSX
>-               defaultset="menubar-items"
>+             defaultset="menubar-items"
> #else
>-               defaultset="menubar-items,spring,throbber-box"
>+             defaultset="menubar-items,spring,updates-item,throbber-box"

Add updates-item to default set

... various whitespace ...

>+      <toolbaritem id="updates-item" title="&updatesItem.title;" align="center" pack="center">
>+        <toolbarbutton type="updates" id="updates"/>
>+      </toolbaritem>
>+

add updates toolbar tem

> #ifdef XP_MACOSX
>-             defaultset="back-button,forward-button,reload-button,stop-button,home-button,urlbar-container,search-container,throbber-box,window-controls"
>+             defaultset="back-button,forward-button,reload-button,stop-button,home-button,urlbar-container,search-container,updates-item,throbber-box,window-controls"
> #else

add updates item to macosx main toolbar

>-    <statusbarpanel id="statusbar-updates"/>

remove statusbar item

>Index: toolkit/locales/en-US/chrome/mozapps/update/update.properties
>-update.app.url=http://update.mozilla.org/update/firefox/en-US.rdf
>+app.update.url=https://update.mozilla.org/update/firefox/en-US.rdf

New (secure) update location.

>+updatesdisabledtitle=Update Disabled
>+updatesdisabledmessage=Update has been disabled by your Administrator.

Update Disabled error strings

>Index: toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties
>-update.extensions.wsdl=http://update.mozilla.org:8080/axis/services/VersionCheck?wsdl
>-extensions.getMoreExtensionsURL=http://update.mozilla.org/extensions/?application=%APPID%
>-extensions.getMoreThemesURL=http://update.mozilla.org/themes/?application=%APPID%
>+extensions.update.url=https://update.mozilla.org/update/VersionCheck.php?reqVersion=%REQ_VERSION%&id=%ITEM_ID%&version=%ITEM_VERSION%&maxAppVersion=%ITEM_MAXAPPVERSION%&appID=%APP_ID%&appVersion=%APP_VERSION%
>+extensions.getMoreExtensionsURL=https://update.mozilla.org/extensions/?application=%APPID%
>+extensions.getMoreThemesURL=https://update.mozilla.org/themes/?application=%APPID%

New secure update urls

>+dssSwitchAfterRestart=Restart %S to use.

DSS is disabled error msg

Comment 9

[Alfred Kayser]

Can someone explain the logic of updateCount and severity attributes of the
toolbarbutton?
The following seems to happen:
if updateCount = 0 then button is hidden (why?, one should be able to still
check for updates)
if severity = 0, 1 or 2 there are updates, with different urgency.

What if both severity is set to 0 and updateCount is 0 then ???
Would it not be better to make the severity 0,1,2,3
with the meaning: 0=no update, 1 - 3: updates with different urgencies...

Signed, an confused themer (nautipolis, walut, littlemozilla, etc).

Comment 10

[(not currently active) Ted Mielczarek]

This landed on the trunk, didn't it?  (Should be FIXED)

Comment 11

[alanjstr]

We use RDF now.  
http://lxr.mozilla.org/mozilla/source/webtools/update/update/VersionCheck.php