Improve handling of server and network errors (bug 335965) r=marria

2006-06-02 06:06:54 +00:00 · 2006-06-02 06:06:54 +00:00 · 85d0b27116
--- a/allmakefiles.sh
+++ b/allmakefiles.sh
@ -880,6 +880,7 @@ extensions/typeaheadfind/Makefile

 MAKEFILES_metrics="
 extensions/metrics/Makefile
+extensions/metrics/build/Makefile
 extensions/metrics/public/Makefile
 extensions/metrics/src/Makefile
 extensions/metrics/test/Makefile
--- a/extensions/metrics/Makefile.in
+++ b/extensions/metrics/Makefile.in
@ -48,7 +48,7 @@ NO_JAR_AUTO_REG		= 1
 INSTALL_EXTENSION_ID	= metrics@mozilla.org
 XPI_PKGNAME		= metrics-$(MOZ_APP_VERSION)

-DIRS		= public src
+DIRS		= public src build

 ifdef ENABLE_TESTS
 TOOL_DIRS += test
--- a/extensions/metrics/build/Makefile.in
+++ b/extensions/metrics/build/Makefile.in
@ -0,0 +1,76 @@
+# vim:set ts=8 sw=8 sts=8 noet:
+# ***** BEGIN LICENSE BLOCK *****
+# Version: MPL 1.1/GPL 2.0/LGPL 2.1
+#
+# The contents of this file are subject to the Mozilla Public License Version
+# 1.1 (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+# http://www.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS IS" basis,
+# WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+# for the specific language governing rights and limitations under the
+# License.
+#
+# The Original Code is the Metrics extension.
+#
+# The Initial Developer of the Original Code is Google Inc.
+# Portions created by the Initial Developer are Copyright (C) 2006
+# the Initial Developer. All Rights Reserved.
+#
+# Contributor(s):
+#   Brian Ryner <bryner@brianryner.com>
+#
+# Alternatively, the contents of this file may be used under the terms of
+# either the GNU General Public License Version 2 or later (the "GPL"), or
+# the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+# in which case the provisions of the GPL or the LGPL are applicable instead
+# of those above. If you wish to allow use of your version of this file only
+# under the terms of either the GPL or the LGPL, and not to allow others to
+# use your version of this file under the terms of the MPL, indicate your
+# decision by deleting the provisions above and replace them with the notice
+# and other provisions required by the GPL or the LGPL. If you do not delete
+# the provisions above, a recipient may use your version of this file under
+# the terms of any one of the MPL, the GPL or the LGPL.
+#
+# ***** END LICENSE BLOCK *****
+
+DEPTH		= ../../..
+topsrcdir	= @top_srcdir@
+srcdir		= @srcdir@
+VPATH		= @srcdir@
+
+include $(DEPTH)/config/autoconf.mk
+
+MODULE		= metrics
+LIBRARY_NAME	= metrics
+SHORT_LIBNAME	= metrics
+
+XPI_NAME	= metrics
+
+REQUIRES	= \
+		xpcom \
+		necko \
+		content \
+		$(NULL)
+
+CPPSRCS		= \
+		nsMetricsModule.cpp \
+		$(NULL)
+
+SHARED_LIBRARY_LIBS = \
+		../src/$(LIB_PREFIX)metrics_s.$(LIB_SUFFIX) \
+		$(DIST)/lib/$(LIB_PREFIX)bz2.$(LIB_SUFFIX) \
+		$(NULL)
+
+EXTRA_DSO_LDOPTS = $(XPCOM_GLUE_LDOPTS) \
+		   $(NSPR_LIBS) \
+		   $(NULL)
+
+LOCAL_INCLUDES	= \
+		-I$(srcdir)/../src \
+		-I$(DIST)/public/nss \
+		-I$(DIST)/private/nss \
+		$(NULL)
+
+include $(topsrcdir)/config/rules.mk
--- a/extensions/metrics/build/nsMetricsModule.cpp
+++ b/extensions/metrics/build/nsMetricsModule.cpp
@ -0,0 +1,128 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim:set ts=2 sw=2 sts=2 et cindent: */
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is the Metrics extension.
+ *
+ * The Initial Developer of the Original Code is Google Inc.
+ * Portions created by the Initial Developer are Copyright (C) 2006
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *  Darin Fisher <darin@meer.net>
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+#include "nsMetricsModule.h"
+#include "nsMetricsService.h"
+#include "nsLoadCollector.h"
+#include "nsWindowCollector.h"
+#include "nsProfileCollector.h"
+#include "nsUICommandCollector.h"
+#include "nsIGenericFactory.h"
+#include "nsICategoryManager.h"
+#include "nsServiceManagerUtils.h"
+#include "nsCOMPtr.h"
+#include "nsXPCOMCID.h"
+#ifndef MOZILLA_1_8_BRANCH
+#include "nsIClassInfoImpl.h"
+#endif
+
+NS_DECL_CLASSINFO(nsMetricsService)
+
+#define COLLECTOR_CONTRACTID(type) \
+  "@mozilla.org/metrics/collector;1?name=" type ":" NS_METRICS_NAMESPACE
+
+static NS_METHOD
+nsMetricsServiceRegisterSelf(nsIComponentManager *compMgr,
+                             nsIFile *path,
+                             const char *loaderStr,
+                             const char *type,
+                             const nsModuleComponentInfo *info)
+{
+  nsCOMPtr<nsICategoryManager> cat =
+      do_GetService(NS_CATEGORYMANAGER_CONTRACTID);
+  NS_ENSURE_STATE(cat);
+
+  cat->AddCategoryEntry("app-startup",
+                        NS_METRICSSERVICE_CLASSNAME,
+                        "service," NS_METRICSSERVICE_CONTRACTID,
+                        PR_TRUE, PR_TRUE, nsnull);
+  return NS_OK;
+}
+
+NS_GENERIC_FACTORY_CONSTRUCTOR_INIT(nsLoadCollector, Init)
+NS_GENERIC_FACTORY_CONSTRUCTOR(nsWindowCollector)
+NS_GENERIC_FACTORY_CONSTRUCTOR(nsProfileCollector)
+NS_GENERIC_FACTORY_CONSTRUCTOR(nsUICommandCollector)
+
+static const nsModuleComponentInfo components[] = {
+  {
+    NS_METRICSSERVICE_CLASSNAME,
+    NS_METRICSSERVICE_CID,
+    NS_METRICSSERVICE_CONTRACTID,
+    nsMetricsService::Create,
+    nsMetricsServiceRegisterSelf,
+    NULL,
+    NULL,
+    NS_CI_INTERFACE_GETTER_NAME(nsMetricsService),
+    NULL,
+    &NS_CLASSINFO_NAME(nsMetricsService),
+    nsIClassInfo::MAIN_THREAD_ONLY | nsIClassInfo::SINGLETON
+  },
+  {
+    NS_METRICSSERVICE_CLASSNAME,
+    NS_METRICSSERVICE_CID,
+    NS_ABOUT_MODULE_CONTRACTID_PREFIX "metrics",
+    nsMetricsService::Create
+  },
+  {
+    NS_LOADCOLLECTOR_CLASSNAME,
+    NS_LOADCOLLECTOR_CID,
+    COLLECTOR_CONTRACTID("document"),
+    nsLoadCollectorConstructor
+  },
+  {
+    NS_WINDOWCOLLECTOR_CLASSNAME,
+    NS_WINDOWCOLLECTOR_CID,
+    COLLECTOR_CONTRACTID("window"),
+    nsWindowCollectorConstructor
+  },
+  {
+    NS_PROFILECOLLECTOR_CLASSNAME,
+    NS_PROFILECOLLECTOR_CID,
+    COLLECTOR_CONTRACTID("profile"),
+    nsProfileCollectorConstructor
+  },
+  {
+    NS_UICOMMANDCOLLECTOR_CLASSNAME,
+    NS_UICOMMANDCOLLECTOR_CID,
+    COLLECTOR_CONTRACTID("uielement"),
+    nsUICommandCollectorConstructor
+  }
+};
+
+NS_IMPL_NSGETMODULE(metrics, components)
--- a/extensions/metrics/src/Makefile.in
+++ b/extensions/metrics/src/Makefile.in
@ -43,9 +43,8 @@ VPATH		= @srcdir@
 include $(DEPTH)/config/autoconf.mk

 MODULE		= metrics
-LIBRARY_NAME	= metrics
-SHORT_LIBNAME	= metrics
-IS_COMPONENT	= 1
+LIBRARY_NAME	= metrics_s
+SHORT_LIBNAME	= metrics_s

 XPI_NAME	= metrics

@ -89,7 +88,6 @@ CPPSRCS		= \
 		nsLoadCollector.cpp \
 		nsMetricsConfig.cpp \
 		nsMetricsEventItem.cpp \
-		nsMetricsModule.cpp \
 		nsMetricsService.cpp \
 		nsProfileCollector.cpp \
 		nsWindowCollector.cpp \
@ -97,11 +95,11 @@ CPPSRCS		= \
 		nsStringUtils.cpp \
 		$(NULL)

-SHARED_LIBRARY_LIBS += $(DIST)/lib/$(LIB_PREFIX)bz2.$(LIB_SUFFIX)
+# Create a static library to link into the component library and unit tests
+FORCE_STATIC_LIB = 1

-EXTRA_DSO_LDOPTS = $(XPCOM_GLUE_LDOPTS) \
-		   $(NSPR_LIBS) \
-		   $(NULL)
+# md5.c requires NSS headers
+LOCAL_INCLUDES = -I$(DIST)/public/nss -I$(DIST)/private/nss

 # Link against the static CRT
 ifeq ($(OS_ARCH)_$(GNU_CC), WINNT_)
@ -115,5 +113,3 @@ include $(topsrcdir)/config/rules.mk

 export:: $(topsrcdir)/security/nss/lib/freebl/md5.c
 	$(INSTALL) $^ .
-
-LOCAL_INCLUDES += -I$(srcdir) -I$(DIST)/public/nss -I$(DIST)/private/nss
--- a/extensions/metrics/src/nsMetricsConfig.cpp
+++ b/extensions/metrics/src/nsMetricsConfig.cpp
@ -43,11 +43,12 @@
 #include "nsIDOMElement.h"
 #include "nsIDOM3Node.h"
 #include "nsIFileStreams.h"
-#include "nsIFile.h"
+#include "nsILocalFile.h"
 #include "nsComponentManagerUtils.h"
 #include "nsNetCID.h"
 #include "prprf.h"
 #include "nsTArray.h"
+#include "nsIDOMSerializer.h"

 #define NS_DEFAULT_UPLOAD_INTERVAL_SEC 60 * 5

@ -67,6 +68,19 @@ MakeKey(const nsAString &eventNS, const nsAString &eventName)
  return key;
 }

+static void
+SplitKey(const nsString &key, nsString &eventNS, nsString &eventName)
+{
+  PRInt32 colon = FindChar(key, ':');
+  if (colon == -1) {
+    NS_ERROR("keys from MakeKey should always have a colon");
+    return;
+  }
+
+  eventName = Substring(key, 0, colon);
+  eventNS = Substring(key, colon + 1, key.Length() - colon - 1);
+}
+
 // This method leaves the result value unchanged if a parsing error occurs.
 static void
 ReadIntegerAttr(nsIDOMElement *elem, const nsAString &attrName, PRInt32 *result)
@ -87,7 +101,7 @@ nsMetricsConfig::nsMetricsConfig()
 PRBool
 nsMetricsConfig::Init()
 {
-  if (!mEventSet.Init()) {
+  if (!mEventSet.Init() || !mNSURIToPrefixMap.Init()) {
    return PR_FALSE;
  }
  Reset();
@ -102,8 +116,10 @@ nsMetricsConfig::Reset()
  NS_ASSERTION(mEventSet.IsInitialized(), "nsMetricsConfig::Init not called");

  mEventSet.Clear();
+  mNSURIToPrefixMap.Clear();
  mEventLimit = PR_INT32_MAX;
  mUploadInterval = NS_DEFAULT_UPLOAD_INTERVAL_SEC;
+  mHasConfig = PR_FALSE;
 }

 nsresult
@ -111,8 +127,8 @@ nsMetricsConfig::Load(nsIFile *file)
 {
  // The given file references a XML file with the following structure:
  //
-  // <config xmlns="http://www.mozilla.org/metrics"
-  //         xmlns:foo="http://foo.com/metrics">
+  // <response xmlns="http://www.mozilla.org/metrics">
+  //   <config xmlns:foo="http://foo.com/metrics">
  //     <collectors>
  //       <collector type="ui"/>
  //       <collector type="document"/>
@ -122,6 +138,7 @@ nsMetricsConfig::Load(nsIFile *file)
  //     <limit events="200"/>
  //     <upload interval="600"/>
  //   </config>
+  // </response>

  NS_ASSERTION(mEventSet.IsInitialized(), "nsMetricsConfig::Init not called");

@ -144,20 +161,142 @@ nsMetricsConfig::Load(nsIFile *file)
                          getter_AddRefs(doc));
  NS_ENSURE_STATE(doc);

+  // Now, walk the DOM.  Most elements are optional, but we check the root
+  // element to make sure it's a valid response document.
+  nsCOMPtr<nsIDOMElement> elem;
+  doc->GetDocumentElement(getter_AddRefs(elem));
+  NS_ENSURE_STATE(elem);
+
+  nsString nameSpace;
+  elem->GetNamespaceURI(nameSpace);
+  if (!nameSpace.Equals(NS_LITERAL_STRING(NS_METRICS_NAMESPACE))) {
+    // We have a root element, but it's the wrong namespace
+    return NS_ERROR_FAILURE;
+  }
+
+  nsString tag;
+  elem->GetLocalName(tag);
+  if (!tag.Equals(NS_LITERAL_STRING("response"))) {
+    // The root tag isn't what we're expecting
+    return NS_ERROR_FAILURE;
+  }
+
  // At this point, we can clear our old configuration.
  Reset();

-  // Now, walk the DOM.  All config elements are optional, and we ignore stuff
-  // that we don't understand.
-  nsCOMPtr<nsIDOMElement> elem;
-  doc->GetDocumentElement(getter_AddRefs(elem));
-  if (!elem)
-    return NS_OK;
-
  ForEachChildElement(elem, &nsMetricsConfig::ProcessToplevelElement);
  return NS_OK;
 }

+nsresult
+nsMetricsConfig::Save(nsILocalFile *file)
+{
+  nsCOMPtr<nsIDOMDocument> doc =
+    do_CreateInstance("@mozilla.org/xml/xml-document;1");
+  NS_ENSURE_STATE(doc);
+
+  nsCOMPtr<nsIDOMElement> response;
+  nsMetricsUtils::CreateElement(doc, NS_LITERAL_STRING("response"),
+                                getter_AddRefs(response));
+  NS_ENSURE_STATE(response);
+
+  nsCOMPtr<nsIDOMElement> config;
+  nsMetricsUtils::CreateElement(doc, NS_LITERAL_STRING("config"),
+                                getter_AddRefs(config));
+  NS_ENSURE_STATE(config);
+
+  nsCOMPtr<nsIDOMElement> collectors;
+  nsMetricsUtils::CreateElement(doc, NS_LITERAL_STRING("collectors"),
+                                getter_AddRefs(collectors));
+  NS_ENSURE_STATE(collectors);
+
+  nsTArray<nsString> events;
+  GetEvents(events);
+
+  nsCOMPtr<nsIDOMNode> nodeOut;
+  nsresult rv;
+
+  for (PRUint32 i = 0; i < events.Length(); ++i) {
+    nsString eventNS, eventName;
+    SplitKey(events[i], eventNS, eventName);
+
+    nsString prefix;
+    if (!eventNS.Equals(NS_LITERAL_STRING(NS_METRICS_NAMESPACE))) {
+      if (!mNSURIToPrefixMap.Get(eventNS, &prefix)) {
+        MS_LOG(("uri %s not in prefix map",
+                NS_ConvertUTF16toUTF8(eventNS).get()));
+        continue;
+      }
+
+      // Declare the namespace prefix on the root element
+      nsString attrName(NS_LITERAL_STRING("xmlns:"));
+      attrName.Append(prefix);
+      response->SetAttribute(attrName, eventNS);
+    }
+
+    nsCOMPtr<nsIDOMElement> collector;
+    nsMetricsUtils::CreateElement(doc, NS_LITERAL_STRING("collector"),
+                                  getter_AddRefs(collector));
+    NS_ENSURE_STATE(collector);
+
+    nsString shortName;
+    if (!prefix.IsEmpty()) {
+      shortName = prefix;
+      shortName.Append(':');
+    }
+    shortName.Append(eventName);
+
+    collector->SetAttribute(NS_LITERAL_STRING("type"), eventName);
+    collectors->AppendChild(collector, getter_AddRefs(nodeOut));
+  }
+  config->AppendChild(collectors, getter_AddRefs(nodeOut));
+
+  if (mEventLimit != PR_INT32_MAX) {
+    nsCOMPtr<nsIDOMElement> limit;
+    nsMetricsUtils::CreateElement(doc, NS_LITERAL_STRING("limit"),
+                                  getter_AddRefs(limit));
+    NS_ENSURE_STATE(limit);
+
+    nsString limitStr;
+    AppendInt(limitStr, mEventLimit);
+    limit->SetAttribute(NS_LITERAL_STRING("events"), limitStr);
+    config->AppendChild(limit, getter_AddRefs(nodeOut));
+  }
+
+  nsCOMPtr<nsIDOMElement> upload;
+  nsMetricsUtils::CreateElement(doc, NS_LITERAL_STRING("upload"),
+                                getter_AddRefs(upload));
+  NS_ENSURE_STATE(upload);
+
+  nsString intervalStr;
+  AppendInt(intervalStr, mUploadInterval);
+  upload->SetAttribute(NS_LITERAL_STRING("interval"), intervalStr);
+  config->AppendChild(upload, getter_AddRefs(nodeOut));
+
+  response->AppendChild(config, getter_AddRefs(nodeOut));
+
+  nsCOMPtr<nsIDOMSerializer> ds =
+    do_CreateInstance(NS_XMLSERIALIZER_CONTRACTID);
+  NS_ENSURE_STATE(ds);
+
+  nsString docText;
+  rv = ds->SerializeToString(response, docText);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  NS_ConvertUTF16toUTF8 utf8Doc(docText);
+  PRInt32 num = utf8Doc.Length();
+
+  PRFileDesc *fd;
+  rv = file->OpenNSPRFileDesc(
+      PR_WRONLY | PR_CREATE_FILE | PR_TRUNCATE, 0600, &fd);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  PRBool success = (PR_Write(fd, utf8Doc.get(), num) == num);
+  PR_Close(fd);
+
+  return success ? NS_OK : NS_ERROR_FAILURE;
+}
+
 void
 nsMetricsConfig::ForEachChildElement(nsIDOMElement *elem,
                                     ForEachChildElementCallback callback)
@ -183,6 +322,19 @@ nsMetricsConfig::ProcessToplevelElement(nsIDOMElement *elem)
 {
  // Process a top-level element

+  nsString name;
+  elem->GetLocalName(name);
+  if (name.Equals(NS_LITERAL_STRING("config"))) {
+    mHasConfig = PR_TRUE;
+    ForEachChildElement(elem, &nsMetricsConfig::ProcessConfigChild);
+  }
+}
+
+void
+nsMetricsConfig::ProcessConfigChild(nsIDOMElement *elem)
+{
+  // Process a config element child
+
  nsString name;
  elem->GetLocalName(name);
  if (name.Equals(NS_LITERAL_STRING("collectors"))) {
@ -223,8 +375,12 @@ nsMetricsConfig::ProcessCollectorElement(nsIDOMElement *elem)
    // value is the EventName
  } else {
    // value is NamespacePrefix + ":" + EventName
-    node->LookupNamespaceURI(StringHead(type, colon), namespaceURI);
+    nsString prefix(StringHead(type, colon));
+    node->LookupNamespaceURI(prefix, namespaceURI);
    type.Cut(0, colon + 1);
+
+    // Add this namespace -> prefix mapping to our lookup table
+    mNSURIToPrefixMap.Put(namespaceURI, prefix);
  }

  mEventSet.PutEntry(MakeKey(namespaceURI, type));
@ -238,6 +394,26 @@ nsMetricsConfig::IsEventEnabled(const nsAString &eventNS,
  return mEventSet.GetEntry(MakeKey(eventNS, eventName)) != nsnull;
 }

+void
+nsMetricsConfig::SetEventEnabled(const nsAString &eventNS,
+                                 const nsAString &eventName, PRBool enabled)
+{
+  NS_ASSERTION(mEventSet.IsInitialized(), "nsMetricsConfig::Init not called");
+  nsString key = MakeKey(eventNS, eventName);
+  if (enabled) {
+    mEventSet.PutEntry(key);
+  } else {
+    mEventSet.RemoveEntry(key);
+  }
+}
+
+void
+nsMetricsConfig::ClearEvents()
+{
+  NS_ASSERTION(mEventSet.IsInitialized(), "nsMetricsConfig::Init not called");
+  mEventSet.Clear();
+}
+
 /* static */ PLDHashOperator PR_CALLBACK
 nsMetricsConfig::CopyKey(nsStringHashKey *entry, void *userData)
 {
--- a/extensions/metrics/src/nsMetricsConfig.h
+++ b/extensions/metrics/src/nsMetricsConfig.h
@ -39,11 +39,12 @@
 #ifndef nsMetricsConfig_h__
 #define nsMetricsConfig_h__

-#include "nsTHashtable.h"
+#include "nsDataHashtable.h"
 #include "nsHashKeys.h"

 class nsIDOMElement;
 class nsIFile;
+class nsILocalFile;
 template<class E> class nsTArray;

 class nsMetricsConfig
@ -67,6 +68,11 @@ public:
   */
  nsresult Load(nsIFile *file);

+  /**
+   * Writes the current metrics configuration to disk.
+   */
+  nsresult Save(nsILocalFile *file);
+
  /**
   * Call this method to determine if the given event type is enabled for
   * collection.
@ -74,12 +80,23 @@ public:
  PRBool IsEventEnabled(const nsAString &eventNS,
                        const nsAString &eventName) const;

+  /**
+   * Sets a particular event to be enabled or disabled.
+   */
+  void SetEventEnabled(const nsAString &eventNS,
+                       const nsAString &eventName, PRBool enabled);
+
  /**
   * Call this method to get a list of all events that are enabled.
   * The event names are prefixed with the namespace, separated by a colon.
   */
  void GetEvents(nsTArray<nsString> &events);

+  /**
+   * Clears the set of events in this config.
+   */
+  void ClearEvents();
+
  /**
   * Get the limit on the number of events that should be collected.
   */
@ -89,6 +106,15 @@ public:
    return mEventLimit;
  }

+  /**
+   * Sets the event limit.
+   */
+  void SetEventLimit(PRInt32 limit) {
+    NS_ASSERTION(mEventSet.IsInitialized(),
+                 "nsMetricsConfig::Init not called");
+    mEventLimit = limit;
+  }
+
  /**
   * Get the upload interval (measured in seconds).
   */
@ -107,6 +133,15 @@ public:
    mUploadInterval = uploadInterval;
  }

+  /**
+   * Returns true if there was a <config> present in the response.
+   */
+  PRBool HasConfig() const {
+    NS_ASSERTION(mEventSet.IsInitialized(),
+                 "nsMetricsConfig::Init not called");
+    return mHasConfig;
+  }
+
 private:
  typedef void (nsMetricsConfig::*ForEachChildElementCallback)(nsIDOMElement *);

@ -114,14 +149,17 @@ private:
  void ForEachChildElement(nsIDOMElement *elem, ForEachChildElementCallback cb);

  void ProcessToplevelElement(nsIDOMElement *elem);
+  void ProcessConfigChild(nsIDOMElement *elem);
  void ProcessCollectorElement(nsIDOMElement *elem);

  static PLDHashOperator PR_CALLBACK CopyKey(nsStringHashKey *key,
                                             void *userData);

  nsTHashtable<nsStringHashKey> mEventSet;
+  nsDataHashtable<nsStringHashKey,nsString> mNSURIToPrefixMap;
  PRInt32 mEventLimit;
  PRInt32 mUploadInterval;
+  PRBool mHasConfig;
 };

 #endif  // nsMetricsConfig_h__
--- a/extensions/metrics/src/nsMetricsModule.cpp
+++ b/extensions/metrics/src/nsMetricsModule.cpp
--- a/extensions/metrics/src/nsMetricsService.cpp
+++ b/extensions/metrics/src/nsMetricsService.cpp
@ -550,14 +550,8 @@ nsMetricsService::Upload()
  if (NS_SUCCEEDED(rv))
    mUploading = PR_TRUE;

-  // Since UploadData is uploading a copy of the data, we can delete the
-  // original data file, and allow new events to be logged to a new file.
-  nsCOMPtr<nsILocalFile> dataFile;
-  GetDataFile(&dataFile);
-  if (dataFile) {
-    if (NS_FAILED(dataFile->Remove(PR_FALSE)))
-      NS_WARNING("failed to remove data file");
-  }
+  // We keep the original data file until we know we've uploaded
+  // successfully, or get a 4xx (bad request) response from the server.

  // Reset event count and persist.
  mEventCount = 0;
@ -633,7 +627,7 @@ nsMetricsService::OnStartRequest(nsIRequest *request, nsISupports *context)
  NS_ENSURE_STATE(!mConfigOutputStream);

  nsCOMPtr<nsIFile> file;
-  GetConfigFile(getter_AddRefs(file));
+  GetConfigTempFile(getter_AddRefs(file));

  nsCOMPtr<nsIFileOutputStream> out =
      do_CreateInstance(NS_LOCALFILEOUTPUTSTREAM_CONTRACTID);
@ -647,61 +641,150 @@ nsMetricsService::OnStartRequest(nsIRequest *request, nsISupports *context)
  return NS_OK;
 }

+PRBool
+nsMetricsService::LoadNewConfig(nsIFile *newConfig, nsIFile *oldConfig)
+{
+  // Try to load the new config
+  PRBool exists = PR_FALSE;
+  newConfig->Exists(&exists);
+  if (exists && NS_SUCCEEDED(mConfig.Load(newConfig))) {
+    MS_LOG(("Successfully loaded new config"));
+
+    // Replace the old config file with the new one
+    oldConfig->Remove(PR_FALSE);
+
+    nsString filename;
+    oldConfig->GetLeafName(filename);
+
+    nsCOMPtr<nsIFile> directory;
+    oldConfig->GetParent(getter_AddRefs(directory));
+
+    newConfig->MoveTo(directory, filename);
+    return PR_TRUE;
+  }
+
+  MS_LOG(("Couldn't load new config"));
+
+  // We want to disable collection until the next upload interval,
+  // but we don't want to reset the upload interval to the default
+  // if the server had supplied one.  So, write out a new config
+  // that just has the collectors disabled.
+  mConfig.ClearEvents();
+
+  nsCOMPtr<nsILocalFile> lf = do_QueryInterface(oldConfig);
+  nsresult rv = mConfig.Save(lf);
+  if (NS_FAILED(rv)) {
+    MS_LOG(("failed to save config: %d", rv));
+  }
+
+  return PR_FALSE;
+}
+
+void
+nsMetricsService::RemoveDataFile()
+{
+  nsCOMPtr<nsILocalFile> dataFile;
+  GetDataFile(&dataFile);
+  if (!dataFile) {
+    MS_LOG(("Couldn't get data file to remove"));
+    return;
+  }
+
+  nsresult rv = dataFile->Remove(PR_FALSE);
+  if (NS_SUCCEEDED(rv)) {
+    MS_LOG(("Removed data file"));
+  } else {
+    MS_LOG(("Couldn't remove data file: %d", rv));
+  }
+}
+
 NS_IMETHODIMP
 nsMetricsService::OnStopRequest(nsIRequest *request, nsISupports *context,
                                nsresult status)
 {
  MS_LOG(("OnStopRequest status = %x", status));

+  // Close the output stream for the download
  if (mConfigOutputStream) {
    mConfigOutputStream->Close();
    mConfigOutputStream = 0;
  }

-  // Load configuration file:
-
-  nsCOMPtr<nsIFile> file;
-  GetConfigFile(getter_AddRefs(file));
-
-
-  // If the upload fails, we'll first retry at the last upload interval
-  // we saw.  This is useful in cases where the failure is transient.
-  // If we fail kMaxRetries times, we'll defer trying again for a randomly
-  // selected length of time 12-36 hours from the last attempt.
-  // When the 12-36 hour deferred upload is attempted, we reset the state
-  // and will again retry up to kMaxRetriesTimes at the default upload
-  // interval.
+  // There are several possible outcomes of our upload request:
+  // 1. The server returns 200 OK
+  //    We consider the upload a success and delete the old data file.
  //
-  // Any time an upload is successful, the failure count is reset to 0.
+  // 2. The server returns a 4xx error
+  //    There was a problem with the uploaded data, so we delete the data file.
+  //
+  // 3. The server returns a 5xx error
+  //    There was a transient server-side problem.  We keep the data file.
+  //
+  // In any of these cases, we parse the server response.  If it contains
+  // a <config>, then it replaces our current config file.  If not, we reset
+  // to the default configuration, but preserve the upload interval.
+  // Currently we don't properly handle a 3xx response, it's treated like
+  // a 4xx error (delete the data file).
+  //
+  // 4. A network error occurs (NS_FAILED(status) is true)
+  //    We keep the old data and the old config.
+  //
+  // In any of the error cases, we increment the retry count and schedule
+  // a retry for the next upload interval.  To start off, the retry is at
+  // the upload interval specified by our config.  If we fail kMaxRetries
+  // times, we'll delete the data file and defer trying again until a randomly
+  // selected time 12-36 hours from the last attempt.  When the 12-36 hour
+  // deferred upload is attempted, we reset the state and will again retry up
+  // to kMaxRetriesTimes at the default upload interval.
+  //
+  // Any time an upload is successful, the retry count is reset to 0.

-  PRBool success = NS_SUCCEEDED(status);
-  if (success) {
+  nsCOMPtr<nsIFile> configTempFile;  // the response we just downloaded
+  GetConfigTempFile(getter_AddRefs(configTempFile));
+  NS_ENSURE_STATE(configTempFile);
+
+  nsCOMPtr<nsIFile> configFile;  // our old config
+  GetConfigFile(getter_AddRefs(configFile));
+  NS_ENSURE_STATE(configFile);
+
+  PRBool success = PR_FALSE, replacedConfig = PR_FALSE;
+  if (NS_SUCCEEDED(status)) {
+    // If the request succeeded (200), we remove the old data file
+    PRUint32 responseCode = 500;
    nsCOMPtr<nsIHttpChannel> channel = do_QueryInterface(request);
-    NS_ENSURE_STATE(channel);
-    channel->GetRequestSucceeded(&success);
-#ifdef PR_LOGGING
-    PRUint32 responseCode;
+    if (channel) {
      channel->GetResponseStatus(&responseCode);
-    MS_LOG(("Server response code: %lu, success = %d", responseCode, success));
-#endif
+    }
+    MS_LOG(("Server response: %u", responseCode));
+
+    if (responseCode == 200) {
+      success = PR_TRUE;
+      RemoveDataFile();
+    } else if (responseCode < 500) {
+      // This was a request error, so delete the data file
+      RemoveDataFile();
    }

+    replacedConfig = LoadNewConfig(configTempFile, configFile);
+  } else {
+    MS_LOG(("Request failed"));
+  }
+
+  // Clean up the temp file if we didn't rename it
+  if (!replacedConfig) {
+    configTempFile->Remove(PR_FALSE);
+  }
+
+  // Handle success or failure of the request
  if (success) {
-    MS_LOG(("Successful upload"));
-    success = NS_SUCCEEDED(mConfig.Load(file));
-    if (success) {
-      MS_LOG(("Read config file successfully, reset retry count to 0"));
    mRetryCount = 0;
-    }
-  }

-  if (!success) {
-    PRInt32 interval = mConfig.UploadInterval();
-    mConfig.Reset();
-    mConfig.SetUploadInterval(interval);
+    // Clear the next-upload-time pref, in case it was set somehow.
+    FlushClearPref(kUploadTimePref);
+    MS_LOG(("Uploaded successfully and reset retry count"));
+  } else if (++mRetryCount >= kMaxRetries) {
+    RemoveDataFile();

-    MS_LOG(("Failed to upload"));
-    if (++mRetryCount >= kMaxRetries) {
    static const int kSecondsPerHour = 60 * 60;
    mRetryCount = 0;

@ -713,8 +796,7 @@ nsMetricsService::OnStopRequest(nsIRequest *request, nsISupports *context,
    // If we couldn't get any random bytes, just use the default of
    // 12 hours.

-      FlushIntPref(kUploadTimePref,
-                   (PR_Now() / PR_USEC_PER_SEC) + interval_sec);
+    FlushIntPref(kUploadTimePref, (PR_Now() / PR_USEC_PER_SEC) + interval_sec);

    MS_LOG(("Reached max retry count, deferring upload for %d seconds",
            interval_sec));
@ -722,13 +804,8 @@ nsMetricsService::OnStopRequest(nsIRequest *request, nsISupports *context,
    // InitUploadTimer().
  }

-    if (file && NS_FAILED(file->Remove(PR_FALSE)))
-      NS_WARNING("failed to remove config file");
-  }
-
  // Restart the upload timer for our next upload
  InitUploadTimer(PR_FALSE);
-
  EnableCollectors();

  mUploading = PR_FALSE;
@ -1074,8 +1151,7 @@ nsMetricsService::CreateRoot()
 {
  nsresult rv;
  nsCOMPtr<nsIDOMElement> root;
-  rv = mDocument->CreateElementNS(NS_LITERAL_STRING(NS_METRICS_NAMESPACE),
-                                  NS_LITERAL_STRING("log"),
+  rv = nsMetricsUtils::CreateElement(mDocument, NS_LITERAL_STRING("log"),
                                     getter_AddRefs(root));
  NS_ENSURE_SUCCESS(rv, rv);

@ -1363,6 +1439,18 @@ nsMetricsService::GetConfigFile(nsIFile **result)
  file.swap(*result);
 }

+void
+nsMetricsService::GetConfigTempFile(nsIFile **result)
+{
+  nsCOMPtr<nsIFile> file;
+  NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, getter_AddRefs(file));
+  if (file)
+    file->AppendNative(NS_LITERAL_CSTRING("metrics-config.tmp"));
+
+  *result = nsnull;
+  file.swap(*result);
+}
+
 nsresult
 nsMetricsService::GenerateClientID(nsCString &clientID)
 {
@ -1551,3 +1639,11 @@ nsMetricsUtils::GetRandomNoise(void *buf, PRSize size)
  }
  return PR_TRUE;
 }
+
+/* static */ nsresult
+nsMetricsUtils::CreateElement(nsIDOMDocument *ownerDoc,
+                              const nsAString &tag, nsIDOMElement **element)
+{
+  return ownerDoc->CreateElementNS(NS_LITERAL_STRING(NS_METRICS_NAMESPACE),
+                                   tag, element);
+}
--- a/extensions/metrics/src/nsMetricsService.h
+++ b/extensions/metrics/src/nsMetricsService.h
@ -177,6 +177,10 @@ private:
  // A reference to the local file containing our current configuration
  void GetConfigFile(nsIFile **result);

+  // A reference to the local file where we'll download the server response.
+  // We don't replace the real config file until we know the new one is valid.
+  void GetConfigTempFile(nsIFile **result);
+
  // Generate a new random client id string
  nsresult GenerateClientID(nsCString &clientID);

@ -201,6 +205,16 @@ private:
  // Does the real work of GetWindowID().
  PRUint32 GetWindowIDInternal(nsIDOMWindow *window);

+  // Tries to load a new config.  If successful, the old config file is
+  // replaced with the new one.  If the new config couldn't be loaded,
+  // a config file is written which disables collection and preserves the
+  // upload interval from the old config.  Returns true if the new config
+  // file was loaded successfully.
+  PRBool LoadNewConfig(nsIFile *newConfig, nsIFile *oldConfig);
+
+  // Removes the existing data file (metrics.xml)
+  void RemoveDataFile();
+
  static PLDHashOperator PR_CALLBACK
  PruneDisabledCollectors(const nsAString &key,
                          nsCOMPtr<nsIMetricsCollector> &value,
@ -278,6 +292,11 @@ public:
  // from the OS.  Returns true on success, or false if no random
  // bytes are available
  static PRBool GetRandomNoise(void *buf, PRSize size);
+
+  // Creates a new element in the metrics namespace, using the given
+  // ownerDocument and tag.
+  static nsresult CreateElement(nsIDOMDocument *ownerDoc,
+                                const nsAString &tag, nsIDOMElement **element);
 };

 #endif  // nsMetricsService_h__
--- a/extensions/metrics/test/Makefile.in
+++ b/extensions/metrics/test/Makefile.in
@ -43,13 +43,53 @@ include $(DEPTH)/config/autoconf.mk

 MODULE		= test_metrics

+REQUIRES	= \
+		xpcom \
+		metrics \
+		necko \
+		$(NULL)
+
+CPPSRCS		= \
+		TestMetricsConfig.cpp \
+		$(NULL)
+
+SIMPLE_PROGRAMS = $(CPPSRCS:.cpp=$(BIN_SUFFIX))
+
+LOCAL_INCLUDES	= \
+		-I$(srcdir)/../src \
+		-I$(DIST)/public/nss \
+		-I$(DIST)/private/nss \
+		$(NULL)
+
+LIBS		= \
+		../src/$(LIB_PREFIX)metrics_s.$(LIB_SUFFIX) \
+		$(DIST)/lib/$(LIB_PREFIX)bz2.$(LIB_SUFFIX) \
+		$(XPCOM_GLUE_LDOPTS) \
+		$(NSPR_LIBS) \
+		$(NULL)
+
 include $(topsrcdir)/config/rules.mk

 _UNIT_FILES := $(wildcard $(srcdir)/unit/*.js)

+# Give the unit tests absolute paths to the data and temp directories.
+# For cygwin, we need to convert the paths to native Windows paths.
+ifdef CYGWIN_WRAPPER
+TESTDATA_DIR := `cygpath -wa $(srcdir)/data`
+TEST_TMPDIR := `cygpath -wa .`
+else
+TESTDATA_DIR := `cd $(srcdir)/data; pwd`
+TEST_TMPDIR := `pwd`
+endif
+
 libs:: $(_UNIT_FILES)
 	$(INSTALL) $^ $(DIST)/bin/metrics_unit_tests

 check::
+	@echo Running tests...
+	@for f in $(SIMPLE_PROGRAMS); do \
+	  echo $$f; $(RUN_TEST_PROGRAM) $(DIST)/bin/$$f \
+            $(TESTDATA_DIR) $(TEST_TMPDIR); \
+	done
 	$(RUN_TEST_PROGRAM) $(DIST)/bin/test_all.sh \
 		$(DIST)/bin/metrics_unit_tests
--- a/extensions/metrics/test/TestCommon.h
+++ b/extensions/metrics/test/TestCommon.h
@ -0,0 +1,54 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim:set ts=2 sw=2 sts=2 et cindent: */
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is the Metrics extension.
+ *
+ * The Initial Developer of the Original Code is Google Inc.
+ * Portions created by the Initial Developer are Copyright (C) 2006
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *  Brian Ryner <bryner@brianryner.com>
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+// This file defines common macros for C++ unit tests
+
+#define ASSERT_TRUE_RET(cond, ret) \
+  if (!cond) { \
+    fprintf(stderr, "FAILED: %s at %s:%d\n", #cond, __FILE__, __LINE__); \
+    return ret; \
+  }
+
+#define ASSERT_TRUE(cond) \
+  if (!cond) { \
+    fprintf(stderr, "FAILED: %s at %s:%d\n", #cond, __FILE__, __LINE__); \
+    return ; \
+  }
+
+#define ASSERT_SUCCESS(res) ASSERT_TRUE(NS_SUCCEEDED(res))
+#define ASSERT_FALSE(cond) ASSERT_TRUE(! cond)
--- a/extensions/metrics/test/TestMetricsConfig.cpp
+++ b/extensions/metrics/test/TestMetricsConfig.cpp
@ -0,0 +1,172 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim:set ts=2 sw=2 sts=2 et cindent: */
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is the Metrics extension.
+ *
+ * The Initial Developer of the Original Code is Google Inc.
+ * Portions created by the Initial Developer are Copyright (C) 2006
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *  Brian Ryner <bryner@brianryner.com>
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+// Unit test for nsMetricsConfig
+
+#include "TestCommon.h"
+#include "nsMetricsConfig.h"
+#include "nsMetricsService.h"
+#include "nsXPCOM.h"
+#include "nsILocalFile.h"
+
+#include <stdio.h>
+
+// This singleton must exist in any code that links against libmetrics_s.
+// TODO: find a way to declare this in src/ while still allowing it to be
+// visible to nsMetricsModule.
+NS_DECL_CLASSINFO(nsMetricsService)
+
+static int gTotalTests = 0;
+static int gPassedTests = 0;
+
+void TestLoad(const char *testdata_path)
+{
+  ++gTotalTests;
+
+  nsMetricsConfig config;
+  ASSERT_TRUE(config.Init());
+
+  nsCOMPtr<nsILocalFile> dataFile;
+  NS_NewNativeLocalFile(nsDependentCString(testdata_path),
+                        PR_TRUE, getter_AddRefs(dataFile));
+  ASSERT_TRUE(dataFile);
+
+  ASSERT_SUCCESS(dataFile->AppendNative(
+                     NS_LITERAL_CSTRING("test_config.xml")));
+  ASSERT_SUCCESS(config.Load(dataFile));
+
+  ASSERT_TRUE(config.IsEventEnabled(NS_LITERAL_STRING(NS_METRICS_NAMESPACE),
+                                    NS_LITERAL_STRING("foo")));
+  ASSERT_TRUE(config.IsEventEnabled(NS_LITERAL_STRING(NS_METRICS_NAMESPACE),
+                                    NS_LITERAL_STRING("bar")));
+  ASSERT_FALSE(config.IsEventEnabled(NS_LITERAL_STRING(NS_METRICS_NAMESPACE),
+                                     NS_LITERAL_STRING("baz")));
+
+  ASSERT_TRUE(config.EventLimit() == 200);
+  ASSERT_TRUE(config.UploadInterval() == 1000);
+  ASSERT_TRUE(config.HasConfig());
+  ++gPassedTests;
+}
+
+// Returns true if the contents of |file| match |contents|.
+static PRBool CheckFileContents(nsILocalFile *file, const char *contents)
+{
+  nsCString nativePath;
+  file->GetNativePath(nativePath);
+
+  // Now read in the file contents and compare to the expected output
+  PRFileInfo info;
+  ASSERT_TRUE_RET(PR_GetFileInfo(nativePath.get(), &info) == PR_SUCCESS,
+                  PR_FALSE);
+
+  char *buf = new char[info.size + 1];
+  ASSERT_TRUE_RET(buf, PR_FALSE);
+
+  PRFileDesc *fd = PR_Open(nativePath.get(), PR_RDONLY, 0);
+  ASSERT_TRUE_RET(fd, PR_FALSE);
+
+  ASSERT_TRUE_RET(PR_Read(fd, buf, info.size) == info.size, PR_FALSE);
+  PR_Close(fd);
+  buf[info.size] = '\0';
+
+  // Leave the file in place if the test failed
+  ASSERT_TRUE_RET(!strcmp(buf, contents), PR_FALSE);
+  PR_Delete(nativePath.get());
+  delete[] buf;
+  return PR_TRUE;
+}
+
+void TestSave(const char *temp_data_path)
+{
+  ++gTotalTests;
+  static const char kFilename[] = "test-save.xml";
+  static const char kExpectedContents[] =
+    "<response xmlns=\"http://www.mozilla.org/metrics\"><config>"
+    "<collectors>"
+      "<collector type=\"uielement\"/>"
+    "</collectors>"
+    "<limit events=\"300\"/>"
+    "<upload interval=\"500\"/>"
+    "</config></response>";
+
+  nsMetricsConfig config;
+  ASSERT_TRUE(config.Init());
+
+  // The data file goes to the current directory
+  config.SetEventEnabled(NS_LITERAL_STRING(NS_METRICS_NAMESPACE),
+                         NS_LITERAL_STRING("uielement"), PR_TRUE);
+  config.SetUploadInterval(500);
+  config.SetEventLimit(300);
+
+  nsCOMPtr<nsILocalFile> outFile;
+  NS_NewNativeLocalFile(nsDependentCString(temp_data_path),
+                        PR_TRUE, getter_AddRefs(outFile));
+  ASSERT_TRUE(outFile);
+  ASSERT_SUCCESS(outFile->AppendNative(nsDependentCString(kFilename)));
+
+  ASSERT_SUCCESS(config.Save(outFile));
+  ASSERT_TRUE(CheckFileContents(outFile, kExpectedContents));
+
+  // Now test with no collectors
+  static const char kExpectedOutputNoEvents[] =
+    "<response xmlns=\"http://www.mozilla.org/metrics\"><config>"
+    "<collectors/>"
+    "<limit events=\"300\"/>"
+    "<upload interval=\"500\"/>"
+    "</config></response>";
+
+  config.ClearEvents();
+  ASSERT_SUCCESS(config.Save(outFile));
+  ASSERT_TRUE(CheckFileContents(outFile, kExpectedOutputNoEvents));
+
+  ++gPassedTests;
+}
+
+int main(int argc, char **argv)
+{
+  if (argc < 2) {
+    fprintf(stderr, "Usage: %s test_data_path temp_data_path\n", argv[0]);
+    return 1;
+  }
+
+  TestLoad(argv[1]);
+  TestSave(argv[2]);
+
+  printf("%d/%d tests passed\n", gPassedTests, gTotalTests);
+  return 0;
+}
--- a/extensions/metrics/test/data/test_config.xml
+++ b/extensions/metrics/test/data/test_config.xml
@ -0,0 +1 @@
+<response xmlns="http://www.mozilla.org/metrics"><config><collectors><collector type="foo"/><collector type="bar"/></collectors><event limit="200"/><upload interval="1000"/></config></response>
				`@ -0,0 +1 @@`
				`<response xmlns="http://www.mozilla.org/metrics"><config><collectors><collector type="foo"/><collector type="bar"/></collectors><event limit="200"/><upload interval="1000"/></config></response>`