Bug 1194049 - Part 4: clear discovered devices when re-discover; r=schien a=lizzard
☠☠ backed out by 352f17922034 ☠ ☠
authorLiang-Heng Chen <xeonchen@mozilla.com>
Wed, 30 Sep 2015 01:48:00 +0200
changeset 606992 9a761fd458be0d5e354343127fc8408b3f43b416
parent 606991 dfd7ec48d4fe9e5ee3509a6d15540c9a1ffc71d1
child 606993 775552352d7a5d2e704697306228ce6f5a2c66e9
push id93006
push userjyavenard@mozilla.com
push dateThu, 15 Oct 2015 05:15:33 +0000
treeherdertry@45ea2a01301e [default view] [failures only]
reviewersschien, lizzard
bugs1194049
milestone43.0a2
Bug 1194049 - Part 4: clear discovered devices when re-discover; r=schien a=lizzard
dom/presentation/PresentationDeviceManager.cpp
dom/presentation/interfaces/nsIPresentationDeviceManager.idl
dom/presentation/provider/MulticastDNSDeviceProvider.cpp
dom/presentation/provider/MulticastDNSDeviceProvider.h
dom/presentation/tests/xpcshell/test_multicast_dns_device_provider.js
--- a/dom/presentation/PresentationDeviceManager.cpp
+++ b/dom/presentation/PresentationDeviceManager.cpp
@@ -148,16 +148,21 @@ PresentationDeviceManager::GetDeviceAvai
 }
 
 NS_IMETHODIMP
 PresentationDeviceManager::GetAvailableDevices(nsIArray** aRetVal)
 {
   NS_ENSURE_ARG_POINTER(aRetVal);
   MOZ_ASSERT(NS_IsMainThread());
 
+  // Bug 1194049: some providers may discontinue discovery after timeout.
+  // Call |ForceDiscovery()| here to make sure device lists are updated.
+  NS_DispatchToMainThread(
+      NS_NewRunnableMethod(this, &PresentationDeviceManager::ForceDiscovery));
+
   nsCOMPtr<nsIMutableArray> devices = do_CreateInstance(NS_ARRAY_CONTRACTID);
   for (uint32_t i = 0; i < mDevices.Length(); ++i) {
     devices->AppendElement(mDevices[i], false);
   }
 
   devices.forget(aRetVal);
 
   return NS_OK;
--- a/dom/presentation/interfaces/nsIPresentationDeviceManager.idl
+++ b/dom/presentation/interfaces/nsIPresentationDeviceManager.idl
@@ -8,17 +8,17 @@ interface nsIArray;
 interface nsIPresentationDeviceProvider;
 
 %{C++
 #define PRESENTATION_DEVICE_MANAGER_CONTRACTID "@mozilla.org/presentation-device/manager;1"
 #define PRESENTATION_DEVICE_CHANGE_TOPIC "presentation-device-change"
 %}
 
 /*
- * Manager for the device availablility. User can observe "presentation-device-change"
+ * Manager for the device availability. User can observe "presentation-device-change"
  * for any update of the available devices.
  */
 [scriptable, uuid(beb61db5-3d5f-454f-a15a-dbfa0337c569)]
 interface nsIPresentationDeviceManager : nsISupports
 {
   // true if there is any device available.
   readonly attribute boolean deviceAvailable;
 
@@ -36,12 +36,14 @@ interface nsIPresentationDeviceManager :
 
   /*
    * Force all registered device providers to update device information.
    */
   void forceDiscovery();
 
   /*
    * Retrieve all available devices, return a list of nsIPresentationDevice.
+   * The returned list is a cached device list and could be out-of-date.
+   * Observe device change events to get following updates.
    */
   nsIArray getAvailableDevices();
 };
 
--- a/dom/presentation/provider/MulticastDNSDeviceProvider.cpp
+++ b/dom/presentation/provider/MulticastDNSDeviceProvider.cpp
@@ -154,16 +154,18 @@ nsresult
 MulticastDNSDeviceProvider::Uninit()
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (!mInitialized) {
     return NS_OK;
   }
 
+  ClearDevices();
+
   Preferences::RemoveObservers(this, kObservedPrefs);
 
   StopDiscovery(NS_OK);
   UnregisterService(NS_OK);
 
   mMulticastDNS = nullptr;
 
   if (mWrappedListener) {
@@ -239,40 +241,205 @@ MulticastDNSDeviceProvider::UnregisterSe
   }
 
   return NS_OK;
 }
 
 nsresult
 MulticastDNSDeviceProvider::StopDiscovery(nsresult aReason)
 {
+  LOG_I("StopDiscovery (0x%08x)", aReason);
+
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(mDiscoveryTimer);
 
   unused << mDiscoveryTimer->Cancel();
 
   if (mDiscoveryRequest) {
     mDiscoveryRequest->Cancel(aReason);
     mDiscoveryRequest = nullptr;
   }
 
   return NS_OK;
 }
 
+nsresult
+MulticastDNSDeviceProvider::AddDevice(const nsACString& aServiceName,
+                                      const nsACString& aServiceType,
+                                      const nsACString& aHost,
+                                      const uint16_t aPort)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(mPresentationServer);
+
+  nsresult rv;
+
+  nsCOMPtr<nsIPresentationDevice> device;
+  if (NS_WARN_IF(NS_FAILED(rv =
+      mPresentationServer->CreateTCPDevice(aHost, /* ID */
+                                           aServiceName,
+                                           aServiceType,
+                                           aHost,
+                                           aPort,
+                                           getter_AddRefs(device))))) {
+    return rv;
+  }
+
+  nsCOMPtr<nsIPresentationDeviceListener> listener;
+  if (NS_SUCCEEDED(GetListener(getter_AddRefs(listener))) && listener) {
+    unused << listener->AddDevice(device);
+  }
+
+  mDevices.AppendElement(Device(aHost, DeviceState::eActive));
+
+  return NS_OK;
+}
+
+nsresult
+MulticastDNSDeviceProvider::UpdateDevice(const uint32_t aIndex,
+                                         const nsACString& aServiceName,
+                                         const nsACString& aServiceType,
+                                         const nsACString& aHost,
+                                         const uint16_t aPort)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(mPresentationServer);
+
+  if (NS_WARN_IF(aIndex >= mDevices.Length())) {
+    return NS_ERROR_INVALID_ARG;
+  }
+
+  nsresult rv;
+
+  nsCOMPtr<nsIPresentationDevice> device;
+  if (NS_WARN_IF(NS_FAILED(rv =
+      mPresentationServer->UpdateTCPDevice(aHost, /* ID */
+                                           aServiceName,
+                                           aServiceType,
+                                           aHost,
+                                           aPort,
+                                           getter_AddRefs(device))))) {
+    return rv;
+  }
+
+  nsCOMPtr<nsIPresentationDeviceListener> listener;
+  if (NS_SUCCEEDED(GetListener(getter_AddRefs(listener))) && listener) {
+    unused << listener->UpdateDevice(device);
+  }
+
+  mDevices[aIndex].state = DeviceState::eActive;
+
+  return NS_OK;
+}
+
+nsresult
+MulticastDNSDeviceProvider::RemoveDevice(const uint32_t aIndex)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(mPresentationServer);
+
+  if (NS_WARN_IF(aIndex >= mDevices.Length())) {
+    return NS_ERROR_INVALID_ARG;
+  }
+
+  nsCString deviceId = mDevices[aIndex].id;
+  LOG_I("RemoveDevice: %s", deviceId.get());
+
+  nsCOMPtr<nsIPresentationDevice> device;
+  if (NS_FAILED(mPresentationServer->GetTCPDevice(deviceId,
+                                                  getter_AddRefs(device)))) {
+    LOG_I("ignore non-existing device: %s", deviceId.get());
+    return NS_OK;
+  }
+
+  nsresult rv;
+  if (NS_WARN_IF(NS_FAILED(rv = mPresentationServer->RemoveTCPDevice(deviceId)))) {
+    return rv;
+  }
+
+  nsCOMPtr<nsIPresentationDeviceListener> listener;
+  if (NS_SUCCEEDED(GetListener(getter_AddRefs(listener))) && listener) {
+    unused << listener->RemoveDevice(device);
+  }
+
+  mDevices.RemoveElementAt(aIndex);
+  return NS_OK;
+}
+
+bool
+MulticastDNSDeviceProvider::FindDevice(const nsACString& aId,
+                                       uint32_t& aIndex)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  size_t index = mDevices.IndexOf(Device(aId, DeviceState::eUnknown),
+                                  0,
+                                  DeviceIdComparator());
+
+  if (index == mDevices.NoIndex) {
+    return false;
+  }
+
+  aIndex = index;
+  return true;
+}
+
+void
+MulticastDNSDeviceProvider::MarkAllDevicesUnknown()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  for (auto& device : mDevices) {
+    device.state = DeviceState::eUnknown;
+  }
+}
+
+void
+MulticastDNSDeviceProvider::ClearUnknownDevices()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  size_t i = mDevices.Length();
+  while (i > 0) {
+    --i;
+    if (mDevices[i].state == DeviceState::eUnknown) {
+      NS_WARN_IF(NS_FAILED(RemoveDevice(i)));
+    }
+  }
+}
+
+void
+MulticastDNSDeviceProvider::ClearDevices()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  size_t i = mDevices.Length();
+  while (i > 0) {
+    --i;
+    NS_WARN_IF(NS_FAILED(RemoveDevice(i)));
+  }
+}
+
 // nsIPresentationDeviceProvider
 NS_IMETHODIMP
 MulticastDNSDeviceProvider::GetListener(nsIPresentationDeviceListener** aListener)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (NS_WARN_IF(!aListener)) {
     return NS_ERROR_INVALID_POINTER;
   }
 
-  nsCOMPtr<nsIPresentationDeviceListener> listener = do_QueryReferent(mDeviceListener);
+  nsresult rv;
+  nsCOMPtr<nsIPresentationDeviceListener> listener =
+    do_QueryReferent(mDeviceListener, &rv);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
   listener.forget(aListener);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 MulticastDNSDeviceProvider::SetListener(nsIPresentationDeviceListener* aListener)
 {
@@ -299,18 +466,29 @@ MulticastDNSDeviceProvider::ForceDiscove
 {
   LOG_I("ForceDiscovery (%d)", mDiscoveryEnabled);
   MOZ_ASSERT(NS_IsMainThread());
 
   if (!mDiscoveryEnabled) {
     return NS_OK;
   }
 
+  MOZ_ASSERT(mDiscoveryTimer);
   MOZ_ASSERT(mMulticastDNS);
 
+  // if it's already discovering, extend existing discovery timeout.
+  if (mIsDiscovering) {
+    unused << mDiscoveryTimer->Cancel();
+
+    NS_WARN_IF(NS_FAILED(mDiscoveryTimer->Init(this,
+                                               mDiscveryTimeoutMs,
+                                               nsITimer::TYPE_ONE_SHOT)));
+    return NS_OK;
+  }
+
   StopDiscovery(NS_OK);
 
   nsresult rv;
   if (NS_WARN_IF(NS_FAILED(rv = mMulticastDNS->StartDiscovery(
       NS_LITERAL_CSTRING(SERVICE_TYPE),
       mWrappedListener,
       getter_AddRefs(mDiscoveryRequest))))) {
     return rv;
@@ -322,32 +500,40 @@ MulticastDNSDeviceProvider::ForceDiscove
 // nsIDNSServiceDiscoveryListener
 NS_IMETHODIMP
 MulticastDNSDeviceProvider::OnDiscoveryStarted(const nsACString& aServiceType)
 {
   LOG_I("OnDiscoveryStarted");
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(mDiscoveryTimer);
 
+  MarkAllDevicesUnknown();
+
   nsresult rv;
   if (NS_WARN_IF(NS_FAILED(rv = mDiscoveryTimer->Init(this,
                                                       mDiscveryTimeoutMs,
                                                       nsITimer::TYPE_ONE_SHOT)))) {
     return rv;
   }
 
+  mIsDiscovering = true;
+
   return NS_OK;
 }
 
 NS_IMETHODIMP
 MulticastDNSDeviceProvider::OnDiscoveryStopped(const nsACString& aServiceType)
 {
   LOG_I("OnDiscoveryStopped");
   MOZ_ASSERT(NS_IsMainThread());
 
+  ClearUnknownDevices();
+
+  mIsDiscovering = false;
+
   return NS_OK;
 }
 
 NS_IMETHODIMP
 MulticastDNSDeviceProvider::OnServiceFound(nsIDNSServiceInfo* aServiceInfo)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
@@ -364,23 +550,16 @@ MulticastDNSDeviceProvider::OnServiceFou
 
   LOG_I("OnServiceFound: %s", serviceName.get());
 
   if (mRegisteredName == serviceName) {
     LOG_I("ignore self");
     return NS_OK;
   }
 
-  nsCOMPtr<nsIPresentationDevice> device;
-  if (NS_SUCCEEDED(mPresentationServer->GetTCPDevice(serviceName,
-                                                     getter_AddRefs(device)))) {
-    LOG_I("device exists");
-    return NS_OK;
-  }
-
   if (mMulticastDNS) {
     if (NS_WARN_IF(NS_FAILED(rv = mMulticastDNS->ResolveService(
         aServiceInfo, mWrappedListener)))) {
       return rv;
     }
   }
 
   return NS_OK;
@@ -399,28 +578,29 @@ MulticastDNSDeviceProvider::OnServiceLos
 
   nsAutoCString serviceName;
   if (NS_WARN_IF(NS_FAILED(rv = aServiceInfo->GetServiceName(serviceName)))) {
     return rv;
   }
 
   LOG_I("OnServiceLost: %s", serviceName.get());
 
-  nsCOMPtr<nsIPresentationDevice> device;
-  if (NS_FAILED(mPresentationServer->GetTCPDevice(serviceName,
-                                                  getter_AddRefs(device)))) {
-    return NS_OK; // ignore non-existing device;
+  nsAutoCString host;
+  if (NS_WARN_IF(NS_FAILED(rv = aServiceInfo->GetHost(host)))) {
+    return rv;
   }
 
-  NS_WARN_IF(NS_FAILED(mPresentationServer->RemoveTCPDevice(serviceName)));
+  uint32_t index;
+  if (!FindDevice(host, index)) {
+    // given device was not found
+    return NS_OK;
+  }
 
-  nsCOMPtr<nsIPresentationDeviceListener> listener;
-  GetListener(getter_AddRefs(listener));
-  if (listener) {
-    listener->RemoveDevice(device);
+  if (NS_WARN_IF(NS_FAILED(rv = RemoveDevice(index)))) {
+    return rv;
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 MulticastDNSDeviceProvider::OnStartDiscoveryFailed(const nsACString& aServiceType,
                                                    int32_t aErrorCode)
@@ -520,54 +700,43 @@ MulticastDNSDeviceProvider::OnServiceRes
 
   nsAutoCString serviceName;
   if (NS_WARN_IF(NS_FAILED(rv = aServiceInfo->GetServiceName(serviceName)))) {
     return rv;
   }
 
   LOG_I("OnServiceResolved: %s", serviceName.get());
 
-  nsCOMPtr<nsIPresentationDevice> device;
-  nsCOMPtr<nsIPresentationDeviceListener> listener;
-  GetListener(getter_AddRefs(listener));
-
-  if (NS_SUCCEEDED(mPresentationServer->GetTCPDevice(serviceName,
-                                                     getter_AddRefs(device)))) {
-    NS_WARN_IF(NS_FAILED(mPresentationServer->RemoveTCPDevice(serviceName)));
-    if (listener) {
-      NS_WARN_IF(NS_FAILED(listener->RemoveDevice(device)));
-    }
-  }
-
   nsAutoCString host;
   if (NS_WARN_IF(NS_FAILED(rv = aServiceInfo->GetHost(host)))) {
     return rv;
   }
 
   uint16_t port;
   if (NS_WARN_IF(NS_FAILED(rv = aServiceInfo->GetPort(&port)))) {
     return rv;
   }
 
   nsAutoCString serviceType;
   if (NS_WARN_IF(NS_FAILED(rv = aServiceInfo->GetServiceType(serviceType)))) {
     return rv;
   }
 
-  if (NS_WARN_IF(NS_FAILED(rv = mPresentationServer->CreateTCPDevice(serviceName,
-                                                                     serviceName,
-                                                                     serviceType,
-                                                                     host,
-                                                                     port,
-                                                                     getter_AddRefs(device))))) {
-    return rv;
-  }
-
-  if (listener) {
-    listener->AddDevice(device);
+  uint32_t index;
+  if (FindDevice(host, index)) {
+    return UpdateDevice(index,
+                        serviceName,
+                        serviceType,
+                        host,
+                        port);
+  } else {
+    return AddDevice(serviceName,
+                     serviceType,
+                     host,
+                     port);
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 MulticastDNSDeviceProvider::OnResolveFailed(nsIDNSServiceInfo* aServiceInfo,
                                             int32_t aErrorCode)
@@ -665,19 +834,19 @@ MulticastDNSDeviceProvider::OnDiscoverab
   if (mDiscoverable) {
     return RegisterService();
   }
 
   return UnregisterService(NS_OK);
 }
 
 nsresult
-MulticastDNSDeviceProvider::OnServiceNameChanged(const nsCString& aServiceName)
+MulticastDNSDeviceProvider::OnServiceNameChanged(const nsACString& aServiceName)
 {
-  LOG_I("serviceName = %s\n", aServiceName.get());
+  LOG_I("serviceName = %s\n", PromiseFlatCString(aServiceName).get());
   MOZ_ASSERT(NS_IsMainThread());
 
   mServiceName = aServiceName;
 
   nsresult rv;
   if (NS_WARN_IF(NS_FAILED(rv = UnregisterService(NS_OK)))) {
     return rv;
   }
--- a/dom/presentation/provider/MulticastDNSDeviceProvider.h
+++ b/dom/presentation/provider/MulticastDNSDeviceProvider.h
@@ -10,16 +10,17 @@
 #include "nsCOMPtr.h"
 #include "nsICancelable.h"
 #include "nsIDNSServiceDiscovery.h"
 #include "nsIObserver.h"
 #include "nsIPresentationDeviceProvider.h"
 #include "nsITCPPresentationServer.h"
 #include "nsITimer.h"
 #include "nsString.h"
+#include "nsTArray.h"
 #include "nsWeakPtr.h"
 
 namespace mozilla {
 namespace dom {
 namespace presentation {
 
 class DNSServiceWrappedListener;
 class MulticastDNSService;
@@ -41,36 +42,79 @@ public:
   NS_DECL_NSITCPPRESENTATIONSERVERLISTENER
   NS_DECL_NSIOBSERVER
 
   explicit MulticastDNSDeviceProvider() = default;
   nsresult Init();
   nsresult Uninit();
 
 private:
+  enum class DeviceState : uint32_t {
+    eUnknown,
+    eActive
+  };
+
+  struct Device final {
+    explicit Device(const nsACString& aId, DeviceState aState)
+      : id(aId), state(aState)
+    {
+    }
+
+    nsCString id;
+    DeviceState state;
+  };
+
+  struct DeviceIdComparator {
+    bool Equals(const Device& aA, const Device& aB) const {
+      return aA.id == aB.id;
+    }
+  };
+
   virtual ~MulticastDNSDeviceProvider();
   nsresult RegisterService();
   nsresult UnregisterService(nsresult aReason);
   nsresult StopDiscovery(nsresult aReason);
 
+  // device manipulation
+  nsresult AddDevice(const nsACString& aServiceName,
+                     const nsACString& aServiceType,
+                     const nsACString& aHost,
+                     const uint16_t aPort);
+  nsresult UpdateDevice(const uint32_t aIndex,
+                        const nsACString& aServiceName,
+                        const nsACString& aServiceType,
+                        const nsACString& aHost,
+                        const uint16_t aPort);
+  nsresult RemoveDevice(const uint32_t aIndex);
+  bool FindDevice(const nsACString& aId,
+                      uint32_t& aIndex);
+
+  void MarkAllDevicesUnknown();
+  void ClearUnknownDevices();
+  void ClearDevices();
+
+  // preferences
   nsresult OnDiscoveryChanged(bool aEnabled);
   nsresult OnDiscoveryTimeoutChanged(uint32_t aTimeoutMs);
   nsresult OnDiscoverableChanged(bool aEnabled);
-  nsresult OnServiceNameChanged(const nsCString& aServiceName);
+  nsresult OnServiceNameChanged(const nsACString& aServiceName);
 
   bool mInitialized = false;
   nsWeakPtr mDeviceListener;
   nsCOMPtr<nsITCPPresentationServer> mPresentationServer;
   nsCOMPtr<nsIDNSServiceDiscovery> mMulticastDNS;
   nsRefPtr<DNSServiceWrappedListener> mWrappedListener;
 
   nsCOMPtr<nsICancelable> mDiscoveryRequest;
   nsCOMPtr<nsICancelable> mRegisterRequest;
 
+  nsTArray<Device> mDevices;
+
   bool mDiscoveryEnabled = false;
+  bool mIsDiscovering = false;
   uint32_t mDiscveryTimeoutMs;
   nsCOMPtr<nsITimer> mDiscoveryTimer;
 
   bool mDiscoverable = false;
 
   nsCString mServiceName;
   nsCString mRegisteredName;
 };
--- a/dom/presentation/tests/xpcshell/test_multicast_dns_device_provider.js
+++ b/dom/presentation/tests/xpcshell/test_multicast_dns_device_provider.js
@@ -1,11 +1,12 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
+/* global Services, do_register_cleanup, do_test_pending */
 
 "use strict";
 
 const { classes: Cc, interfaces: Ci, manager: Cm, results: Cr, utils: Cu } = Components;
 
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
@@ -43,17 +44,17 @@ MockFactory.prototype = {
 };
 
 function ContractHook(aContractID, aClass) {
   this._contractID = aContractID;
   this.classID = Cc[UUID_CONTRACT_ID].getService(Ci.nsIUUIDGenerator).generateUUID();
   this._newFactory = new MockFactory(aClass);
 
   if (!this.hookedMap.has(this._contractID)) {
-    this.hookedMap.set(this._contractID, new Array());
+    this.hookedMap.set(this._contractID, []);
   }
 
   this.init();
 }
 
 ContractHook.prototype = {
   hookedMap: new Map(), // remember only the most original factory.
 
@@ -153,23 +154,36 @@ MockDNSServiceInfo.prototype = {
     this._attributes = aAttributes;
   },
 
   get attributes() {
     return this._attributes;
   }
 };
 
-function TestPresentationDeviceListener() {}
+function TestPresentationDeviceListener() {
+  this.devices = {};
+}
 TestPresentationDeviceListener.prototype = {
-  QueryInterface: XPCOMUtils.generateQI([Ci.nsIPresentationDeviceListener]),
+  QueryInterface: XPCOMUtils.generateQI([Ci.nsIPresentationDeviceListener,
+                                         Ci.nsISupportsWeakReference]),
 
-  addDevice: function(device) {},
-  removeDevice: function(device) {},
-  updateDevice: function(device) {}
+  addDevice: function(device) { this.devices[device.id] = device; },
+  removeDevice: function(device) { delete this.devices[device.id]; },
+  updateDevice: function(device) { this.devices[device.id] = device; },
+
+  count: function() {
+    var size = 0, key;
+    for (key in this.devices) {
+        if (this.devices.hasOwnProperty(key)) {
+          ++size;
+        }
+    }
+    return size;
+  }
 };
 
 function createDevice(host, port, serviceName, serviceType, domainName, attributes) {
   let device = new MockDNSServiceInfo();
   device.host = host || "";
   device.port = port || 0;
   device.serviceName = serviceName || "";
   device.serviceType = serviceType || "";
@@ -332,36 +346,29 @@ function addDevice() {
                                               mockDevice.port,
                                               mockDevice.serviceName,
                                               mockDevice.serviceType));
     }
   };
 
   let contractHook = new ContractHook(SD_CONTRACT_ID, mockObj);
   let provider = Cc[PROVIDER_CONTRACT_ID].createInstance(Ci.nsIPresentationDeviceProvider);
-  let listener = {
-    QueryInterface: XPCOMUtils.generateQI([Ci.nsIPresentationDeviceListener,
-                                           Ci.nsISupportsWeakReference]),
-    addDevice: function(device) { this.devices.push(device); },
-    removeDevice: function(device) {},
-    updateDevice: function(device) {},
-    devices: []
-  };
-  Assert.equal(listener.devices.length, 0);
+  let listener = new TestPresentationDeviceListener();
+  Assert.equal(listener.count(), 0);
 
   // Start discovery
   provider.listener = listener;
-  Assert.equal(listener.devices.length, 1);
+  Assert.equal(listener.count(), 1);
 
   // Force discovery again
   provider.forceDiscovery();
-  Assert.equal(listener.devices.length, 1);
+  Assert.equal(listener.count(), 1);
 
   provider.listener = null;
-  Assert.equal(listener.devices.length, 1);
+  Assert.equal(listener.count(), 1);
 
   run_next_test();
 }
 
 function noAddDevice() {
   Services.prefs.setBoolPref(PREF_DISCOVERY, false);
 
   let mockDevice = createDevice("device.local", 12345, "service.name", "_mozilla_papi._tcp");
@@ -419,34 +426,201 @@ function addDeviceDynamically() {
                                               mockDevice.port,
                                               mockDevice.serviceName,
                                               mockDevice.serviceType));
     }
   };
 
   let contractHook = new ContractHook(SD_CONTRACT_ID, mockObj);
   let provider = Cc[PROVIDER_CONTRACT_ID].createInstance(Ci.nsIPresentationDeviceProvider);
+  let listener = new TestPresentationDeviceListener();
+  provider.listener = listener;
+  Assert.equal(listener.count(), 0);
+
+  // Enable discovery
+  Services.prefs.setBoolPref(PREF_DISCOVERY, true);
+  Assert.equal(listener.count(), 1);
+
+  // Try discovery again
+  provider.forceDiscovery();
+  Assert.equal(listener.count(), 1);
+
+  // Try discovery once more
+  Services.prefs.setBoolPref(PREF_DISCOVERY, false);
+  Services.prefs.setBoolPref(PREF_DISCOVERY, true);
+  provider.forceDiscovery();
+  Assert.equal(listener.count(), 1);
+
+  provider.listener = null;
+
+  run_next_test();
+}
+
+function updateDevice() {
+  Services.prefs.setBoolPref(PREF_DISCOVERY, true);
+
+  let mockDevice1 = createDevice("A.local", 12345, "N1", "_mozilla_papi._tcp");
+  let mockDevice2 = createDevice("A.local", 23456, "N2", "_mozilla_papi._tcp");
+
+  let mockObj = {
+    discovered: false,
+
+    QueryInterface: XPCOMUtils.generateQI([Ci.nsIDNSServiceDiscovery]),
+    startDiscovery: function(serviceType, listener) {
+      listener.onDiscoveryStarted(serviceType);
+
+      if (!this.discovered) {
+        listener.onServiceFound(mockDevice1);
+      } else {
+        listener.onServiceFound(mockDevice2);
+      }
+      this.discovered = true;
+
+      return {
+        QueryInterface: XPCOMUtils.generateQI([Ci.nsICancelable]),
+        cancel: function() {
+          listener.onDiscoveryStopped(serviceType);
+        }
+      };
+    },
+    registerService: function(serviceInfo, listener) {},
+    resolveService: function(serviceInfo, listener) {
+      Assert.equal(serviceInfo.serviceType, "_mozilla_papi._tcp");
+      if (serviceInfo.serviceName == "N1") {
+        listener.onServiceResolved(mockDevice1);
+      } else if (serviceInfo.serviceName == "N2") {
+        listener.onServiceResolved(mockDevice2);
+      } else {
+        Assert.ok(false);
+      }
+    }
+  };
+
+  let contractHook = new ContractHook(SD_CONTRACT_ID, mockObj);
+  let provider = Cc[PROVIDER_CONTRACT_ID].createInstance(Ci.nsIPresentationDeviceProvider);
   let listener = {
     QueryInterface: XPCOMUtils.generateQI([Ci.nsIPresentationDeviceListener,
                                            Ci.nsISupportsWeakReference]),
-    addDevice: function(device) { this.devices.push(device); },
-    removeDevice: function(device) {},
-    updateDevice: function(device) {},
-    devices: []
+
+    addDevice: function(device) {
+      Assert.ok(!this.isDeviceAdded);
+      Assert.equal(device.id, mockDevice1.host);
+      Assert.equal(device.name, mockDevice1.serviceName);
+      this.isDeviceAdded = true;
+    },
+    removeDevice: function(device) { Assert.ok(false); },
+    updateDevice: function(device) {
+      Assert.ok(!this.isDeviceUpdated);
+      Assert.equal(device.id, mockDevice2.host);
+      Assert.equal(device.name, mockDevice2.serviceName);
+      this.isDeviceUpdated = true;
+    },
+
+    isDeviceAdded: false,
+    isDeviceUpdated: false
   };
-  provider.listener = listener;
-  Assert.equal(listener.devices.length, 0);
+  Assert.equal(listener.isDeviceAdded, false);
+  Assert.equal(listener.isDeviceUpdated, false);
+
+  // Start discovery
+  provider.listener = listener; // discover: N1
+
+  Assert.equal(listener.isDeviceAdded, true);
+  Assert.equal(listener.isDeviceUpdated, false);
+
+  // temporarily disable to stop discovery and re-enable
+  Services.prefs.setBoolPref(PREF_DISCOVERY, false);
+  Services.prefs.setBoolPref(PREF_DISCOVERY, true);
+
+  provider.forceDiscovery(); // discover: N2
+
+  Assert.equal(listener.isDeviceAdded, true);
+  Assert.equal(listener.isDeviceUpdated, true);
+
+  provider.listener = null;
+
+  run_next_test();
+}
+
+function diffDiscovery() {
+  Services.prefs.setBoolPref(PREF_DISCOVERY, true);
+
+  let mockDevice1 = createDevice("A.local", 12345, "N1", "_mozilla_papi._tcp");
+  let mockDevice2 = createDevice("B.local", 23456, "N2", "_mozilla_papi._tcp");
+  let mockDevice3 = createDevice("C.local", 45678, "N3", "_mozilla_papi._tcp");
+
+  let mockObj = {
+    discovered: false,
+
+    QueryInterface: XPCOMUtils.generateQI([Ci.nsIDNSServiceDiscovery]),
+    startDiscovery: function(serviceType, listener) {
+      listener.onDiscoveryStarted(serviceType);
 
-  // Enable discovery
-  Services.prefs.setBoolPref(PREF_DISCOVERY, true);
-  Assert.equal(listener.devices.length, 1);
+      if (!this.discovered) {
+        listener.onServiceFound(mockDevice1);
+        listener.onServiceFound(mockDevice2);
+      } else {
+        listener.onServiceFound(mockDevice1);
+        listener.onServiceFound(mockDevice3);
+      }
+      this.discovered = true;
+
+      return {
+        QueryInterface: XPCOMUtils.generateQI([Ci.nsICancelable]),
+        cancel: function() {
+          listener.onDiscoveryStopped(serviceType);
+        }
+      };
+    },
+    registerService: function(serviceInfo, listener) {},
+    resolveService: function(serviceInfo, listener) {
+      Assert.equal(serviceInfo.serviceType, "_mozilla_papi._tcp");
+      if (serviceInfo.serviceName == "N1") {
+        listener.onServiceResolved(mockDevice1);
+      } else if (serviceInfo.serviceName == "N2") {
+        listener.onServiceResolved(mockDevice2);
+      } else if (serviceInfo.serviceName == "N3") {
+        listener.onServiceResolved(mockDevice3);
+      } else {
+        Assert.ok(false);
+      }
+    }
+  };
 
-  // Try discovery again
-  provider.forceDiscovery();
-  Assert.equal(listener.devices.length, 1);
+  let contractHook = new ContractHook(SD_CONTRACT_ID, mockObj);
+  let provider = Cc[PROVIDER_CONTRACT_ID].createInstance(Ci.nsIPresentationDeviceProvider);
+  let listener = new TestPresentationDeviceListener();
+  Assert.equal(listener.count(), 0);
+
+  // Start discovery
+  provider.listener = listener; // discover: N1, N2
+  Assert.equal(listener.count(), 2);
+  Assert.equal(listener.devices['A.local'].name, mockDevice1.serviceName);
+  Assert.equal(listener.devices['B.local'].name, mockDevice2.serviceName);
+  Assert.ok(!listener.devices['C.local']);
+
+  // temporarily disable to stop discovery and re-enable
+  Services.prefs.setBoolPref(PREF_DISCOVERY, false);
+  Services.prefs.setBoolPref(PREF_DISCOVERY, true);
+
+  provider.forceDiscovery(); // discover: N1, N3, going to remove: N2
+  Assert.equal(listener.count(), 3);
+  Assert.equal(listener.devices['A.local'].name, mockDevice1.serviceName);
+  Assert.equal(listener.devices['B.local'].name, mockDevice2.serviceName);
+  Assert.equal(listener.devices['C.local'].name, mockDevice3.serviceName);
+
+  // temporarily disable to stop discovery and re-enable
+  Services.prefs.setBoolPref(PREF_DISCOVERY, false);
+  Services.prefs.setBoolPref(PREF_DISCOVERY, true);
+
+  provider.forceDiscovery(); // discover: N1, N3, remove: N2
+  Assert.equal(listener.count(), 2);
+  Assert.equal(listener.devices['A.local'].name, mockDevice1.serviceName);
+  Assert.ok(!listener.devices['B.local']);
+  Assert.equal(listener.devices['C.local'].name, mockDevice3.serviceName);
 
   provider.listener = null;
 
   run_next_test();
 }
 
 function serverClosed() {
   Services.prefs.setBoolPref(PREF_DISCOVERABLE, true);
@@ -512,23 +686,23 @@ function serverClosed() {
   Assert.equal(mockObj.serviceUnregistered, 0);
   Assert.equal(listener.devices.length, 1);
 
   let serverListener = provider.QueryInterface(Ci.nsITCPPresentationServerListener);
   serverListener.onClose(Cr.NS_ERROR_UNEXPECTED);
 
   Assert.equal(mockObj.serviceRegistered, 2);
   Assert.equal(mockObj.serviceUnregistered, 1);
-  Assert.equal(listener.devices.length, 2);
+  Assert.equal(listener.devices.length, 1);
 
   // Unregister
   provider.listener = null;
   Assert.equal(mockObj.serviceRegistered, 2);
   Assert.equal(mockObj.serviceUnregistered, 2);
-  Assert.equal(listener.devices.length, 2);
+  Assert.equal(listener.devices.length, 1);
 
   run_next_test();
 }
 
 function run_test() {
   let infoHook = new ContractHook(INFO_CONTRACT_ID, MockDNSServiceInfo);
 
   do_register_cleanup(() => {
@@ -537,12 +711,14 @@ function run_test() {
   });
 
   add_test(registerService);
   add_test(noRegisterService);
   add_test(registerServiceDynamically);
   add_test(addDevice);
   add_test(noAddDevice);
   add_test(addDeviceDynamically);
+  add_test(updateDevice);
+  add_test(diffDiscovery);
   add_test(serverClosed);
 
   run_next_test();
 }