On Fri, Jul 18, 2014 at 2:49 PM, Ehsan Akhgari <ehsan.akhg...@gmail.com> wrote: > On 2014-07-18, 5:28 PM, Dave Hylands wrote: >> >> >> >> ------------------------------------------------------------------------ >> >> *From: *"Ehsan Akhgari" <ehsan.akhg...@gmail.com> >> *To: *"Dave Hylands" <dhyla...@mozilla.com> >> *Cc: *"dev-platform" <dev-platform@lists.mozilla.org> >> *Sent: *Friday, July 18, 2014 2:14:50 PM >> *Subject: *Re: Intent to implement: navigator.deviceStorage >> >> >> On Wed, Jul 16, 2014 at 9:08 PM, Dave Hylands <dhyla...@mozilla.com >> <mailto:dhyla...@mozilla.com>> wrote: >> >> Currently, we have navigator.getDeviceStorage and >> navigator.getDeviceStorages >> >> We're looking to expand device storage to add support for more >> virtual storage areas, like DropBox, or GoogleDrive, etc. >> See bug 1035053 >> >> I was going to propose that we add navigator.deviceStorage (or >> possibly navigator.mozDeviceStorage) and have at least the >> following methods: >> >> deviceStorage.addObserver >> deviceStorage.removeObserver >> >> >> addObserver/removeObserver are Gecko-isms that don't really have a >> counterpart in Web APIs. Why not use an EventListener? >> >> >> No reason, other than I'm not familiar with EventListener. What is an >> EventListener and how would you use it? Maybe just point me at an example? > > > What David and Ralph said! You can grep dom/webidl for EventListener to > find many examples of this. Also, see EventTarget. > > >> deviceStorage.getAll >> deviceStorage.getDefault >> >> We need some new notifications, one to report when the default >> volume has changed (on B2G it is controlled by a setting which >> then gets reflected into a preference), one to report when a new >> storage area (like DropBox) is added, and one to report when it >> goes away. Presumably we'd also need additional APIs to actually >> add/remove storage areas. >> >> deviceStorage.getAll would return exactly what >> navigator.getDeviceStorages returns today, and >> deviceStorage.getDefault would return exactly what >> navigator.getDevicetorages returns today. >> >> I think that we probably need to leave getDeviceStorage and >> getDeviceStorages both around for the time being in order to >> maintain backwards capability. >> >> >> So getAll/getDefault would be exactly like >> getDeviceStorage()/getDeviceStorages? If so, I'm not really sure >> what we gain from this renaming... >> >> >> We need someway to add listeners for the default storage area changing >> and for new storage areas coming and going, and possibly even for an API >> to add/remove storage areas. >> >> So it felt to me that having a new entity called deviceStorage with all >> of the device storage functionality contained within it seemed more >> natural to me than adding several more "free" functions into the >> namespace. I don't really have an opinion one way or the other. I'd just >> like to move things along in a decent direction, but I just don't know >> what that means. > > > Ah I see. I think that your intent is pure, but changing the API like this > will break existing apps that rely on it. Perhaps adding an EventListener > on Window would be enough, so that we can keep the same API? > > (Also, please see Marco's email about this to dev-webapi. It would be nice > if we could coordinate our efforts here.) > > Cheers, > Ehsan > > _______________________________________________ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform
One of the unfortunate design flaws in the current API is the combination of using event listeners and the getDeviceStorage getter returning a new object every time. Once an event listener is added to the object it is rooted for essentially the lifetime of the window because it has to listen to notifications from the filesystem. And you get a new one every time (which you basically have to do since you can only have a single event *handler (as opposed to event listeners)). We've seen automated testing runs that use the phone for a while ending up with hundreds or thousands of rooted DeviceStorage objects. See bug 985197 for the details. I would like to ensure that we don't make the same mistakes again. - Kyle _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
