Justin D'Arcangelo [:justindarc] Reporter
	Description • 9 years ago

Unable to import apps/addons using navigator.mozApps.mgmt.import():

// Retrieve the new tempfile.
var getRequest = sdcard.get(APP_OR_ADDON_FILENAME);
getRequest.onsuccess = () => {
  var addonFile = getRequest.result;

  // Import the app/addon using the tempfile.
  navigator.mozApps.mgmt.import(addonFile)
    .then((addon) => {
      resolve(addon);
    })
    .catch((error) => {
      reject(error);
    });
};

The above code results in the following output in the logcat:

I/Gecko   ( 1590): ##### DeviceStorage: GetVolumeByName('sdcard') failed
I/Gecko   ( 1590): ##### DeviceStorage: GetVolumeByName('sdcard') failed
I/Gecko   ( 1590): ##### DeviceStorage: GetVolumeByName('sdcard') failed

	Justin D'Arcangelo [:justindarc] Reporter
	Comment 1 • 9 years ago

Setting NI? for Dave since he knows DeviceStorage.

	Doug Sherk (:drs) (inactive)
	Comment 2 • 9 years ago

This is busted on both Flame and Aries, at the very least.

	Dave Hylands [:dhylands] Assignee
	Comment 3 • 9 years ago

My preliminary investigation seems to indicate that the ##### DeviceStorage: GetVolumeByName('sdcard') failed messages are coming from the preallocated process.

Could you verify that this is the case for you? (i.e. the 1590 in the error message corresponds to the preallocated process).

What's happening is that when device storage successfully adds or removes a file, it winds up sending out a file-watcher-update notification.

The ##### DeviceStorage: GetVolumeByName('sdcard') failed error messages that I saw were happening when the preallocated process receives the file-watcher-update.

So I need to figure out a way to get the preallocated process to ignore these messages, but I'm pretty sure that this is not causing any problems other than chewing some CPU cycles.

	Justin D'Arcangelo [:justindarc] Reporter
	Comment 4 • 9 years ago

(In reply to Dave Hylands [:dhylands] from comment #3)
> My preliminary investigation seems to indicate that the ##### DeviceStorage:
> GetVolumeByName('sdcard') failed messages are coming from the preallocated
> process.
> 
> Could you verify that this is the case for you? (i.e. the 1590 in the error
> message corresponds to the preallocated process).
> 
> What's happening is that when device storage successfully adds or removes a
> file, it winds up sending out a file-watcher-update notification.
> 
> The ##### DeviceStorage: GetVolumeByName('sdcard') failed error messages
> that I saw were happening when the preallocated process receives the
> file-watcher-update.
> 
> So I need to figure out a way to get the preallocated process to ignore
> these messages, but I'm pretty sure that this is not causing any problems
> other than chewing some CPU cycles.

Confirmed. Getting the following errors logged in logcat:

I/Gecko   (17902): ##### DeviceStorage: GetVolumeByName('sdcard') failed
I/Gecko   (17902): ##### DeviceStorage: GetVolumeByName('sdcard') failed
I/Gecko   (17902): ##### DeviceStorage: GetVolumeByName('sdcard') failed

Then, b2g-ps shows the following:

APPLICATION    SEC USER     PID   PPID  VSIZE  RSS     WCHAN    PC         NAME
b2g              0 root      293   1     260440 87776 ffffffff 00000000 S /system/b2g/b2g
(Nuwa)           0 root      552   293   66952  22940 ffffffff 00000000 S /system/b2g/b2g
Homescreen       2 u0_a1117  1117  552   89768  30076 ffffffff 00000000 S /system/b2g/b2g
Built-in Keyboa  2 u0_a1292  1292  293   90196  31936 ffffffff 00000000 S /system/b2g/plugin-container
Smart Collectio  2 u0_a1462  1462  552   75964  28200 ffffffff 00000000 S /system/b2g/b2g
FM Radio         2 u0_a2020  2020  552   97168  30108 ffffffff 00000000 S /system/b2g/b2g
(Preallocated a  2 u0_a17902 17902 552   71120  18376 ffffffff 00000000 S /system/b2g/b2g

	Dave Hylands [:dhylands] Assignee
	Comment 5 • 9 years ago

Prevents a process which doesn't use device storage from trying to process device storage file update events.

	Alphan Chen [:alchen]
	Comment 6 • 9 years ago

Comment on attachment 8574979 [details] [diff] [review]
Don't try and process DS notifications if the process isn't using DS.

Review of attachment 8574979 [details] [diff] [review]:
-----------------------------------------------------------------

Hi Dave,
I checked the patch. 
There is no concern for this patch.

In my understanding, the error log should be happened when the process receive DS notification.
So process which isn't using DS will hit this problem.
What is the relationship between this problem and the symptom of this bug (Unable to import apps/addons using navigator.mozApps.mgmt.import())?

::: dom/ipc/ContentChild.cpp
@@ +2221,2 @@
>      nsRefPtr<DeviceStorageFile> dsf = new DeviceStorageFile(aStorageType, aStorageName, aPath);
>  

> I/Gecko   ( 1590): ##### DeviceStorage: GetVolumeByName('sdcard') failed
Is the error log happened on this line?

	Dave Hylands [:dhylands] Assignee
	Comment 7 • 9 years ago

(In reply to Alphan Chen [:Alphan] from comment #6)
> Comment on attachment 8574979 [details] [diff] [review]
> Don't try and process DS notifications if the process isn't using DS.
> 
> Review of attachment 8574979 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hi Dave,
> I checked the patch. 
> There is no concern for this patch.
> 
> In my understanding, the error log should be happened when the process
> receive DS notification.
> So process which isn't using DS will hit this problem.
> What is the relationship between this problem and the symptom of this bug
> (Unable to import apps/addons using navigator.mozApps.mgmt.import())?

I don't think that this bug is related. I think that importing addons uses device storage and caused the "##### DeviceStorage: GetVolumeByName('sdcard')" messages.

So this patch addresses the "##### DeviceStorage: GetVolumeByName('sdcard')" messages which were showing up and shouldn't have.

> ::: dom/ipc/ContentChild.cpp
> @@ +2221,2 @@
> >      nsRefPtr<DeviceStorageFile> dsf = new DeviceStorageFile(aStorageType, aStorageName, aPath);
> >  
> 
> > I/Gecko   ( 1590): ##### DeviceStorage: GetVolumeByName('sdcard') failed
> Is the error log happened on this line?

Yeah - In this particular scenario, its the preallocated app which displaying the error messages. And its happening because the preallocated app doesn't yet know what the list of volumes is. So any attempt to instantiate a DeviceStorageFile object will fail.

Once the preallocated app is converted into a real app, it will be given a list of volumes. The particular notification means that some app updated a file on device storage and is telling the other apps about it.

Until an app uses device storage, there isn't any need to process these events.

	Alphan Chen [:alchen]
	Comment 8 • 9 years ago

Comment on attachment 8574979 [details] [diff] [review]
Don't try and process DS notifications if the process isn't using DS.

Review of attachment 8574979 [details] [diff] [review]:
-----------------------------------------------------------------

It looks good for me.

	Dave Hylands [:dhylands] Assignee
	Comment 9 • 9 years ago

https://hg.mozilla.org/integration/b2g-inbound/rev/d93c07e6bcd2

	Dave Hylands [:dhylands] Assignee
	Comment 10 • 9 years ago

Backed out: https://hg.mozilla.org/integration/b2g-inbound/rev/7b56591efd68

seems to be causing: https://treeherder.mozilla.org/logviewer.html#?job_id=1604377&repo=b2g-inbound

	Pulsebot
	Comment 11 • 9 years ago

https://hg.mozilla.org/integration/b2g-inbound/rev/f7f06b74b198

	Dave Hylands [:dhylands] Assignee
	Comment 12 • 9 years ago

Original patch was too aggressive.

Try run with updated patch:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bd7abeb3d389

	Ryan VanderMeulen [:RyanVM]
	Comment 13 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/f7f06b74b198