856798 - Grant storage permission to all apps that use shared/js/mediadb.js or shared/js/async_storage.js

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[David Flanagan [:djf]]

Per recent discussion on dev-gaia, all apps that use indexedDB need the "storage" permission or they will only be able to store 5mb of data.

The mediadb.js and async_storage.js libraries both use IndexedDB. 

This bug is to fix all clients of those two libraries to add the required permission.

Comment 1

[David Flanagan [:djf]]

Daniel,

As you requested on dev-gaia, I'm nominating this for inclusion in tef.

Comment 2

[Maria Angeles Oteo (:oteo)]

For Cost Control and Communications Bug 856557 and bug 856538 
are already resolved and landed in Master. They are also flagged as tef+.

For e-mail, Bug 856732 has already been opened and it's tef+.

Comment 3

[David Flanagan [:djf]]

Apps that use mediadb and async storage:

$ grep -l mediadb.js apps/*/*.html apps/*/*/*.html
apps/gallery/index.html
apps/music/index.html
apps/video/index.html

$ grep -l async_storage.js apps/*/*.html apps/*/*/*.html
apps/camera/index.html
apps/clock/index.html
apps/costcontrol/fte.html
apps/costcontrol/handle_gaps.html
apps/costcontrol/index.html
apps/costcontrol/message_handler.html
apps/costcontrol/widget.html
apps/fm/index.html
apps/music/index.html
apps/music/open.html
apps/settings/index.html
apps/sms/index.html
apps/system/index.html
apps/communications/contacts/fb_link.html
apps/communications/contacts/import.html
apps/communications/contacts/index.html
apps/communications/dialer/index.html
apps/communications/dialer/oncall.html
apps/communications/facebook/fb_oauth.html
apps/communications/facebook/fb_sync.html
apps/communications/ftu/index.html
apps/system/camera/index.html

Comment 4

[David Flanagan [:djf]]

Apps that already have the storage permission:

$ grep -l '"storage"' apps/*/manifest.webapp
apps/communications/manifest.webapp
apps/costcontrol/manifest.webapp
apps/email/manifest.webapp
apps/system/manifest.webapp

Comment 5

[David Flanagan [:djf]]

Apps that use mediadb or async storage and do not already have storage permission: gallery, music, video, camera, clock, fm, settings, sms

Comment 6

[David Flanagan [:djf]]

Attachment: link to patch on github

Comment 7

[David Flanagan [:djf]]

landed on master

Comment 8

[David Flanagan [:djf]]

landed on master: https://github.com/mozilla-b2g/gaia/commit/a1c8f555cdab3ed88860b4b0dfbd189f4bb36d20

Comment 9

[David Flanagan [:djf]]

landed on v1-train: https://github.com/mozilla-b2g/gaia/commit/28521b5a5850fe86d4409d39180a8af335926748

Comment 10

[David Flanagan [:djf]]

landed on v1.0.1: https://github.com/mozilla-b2g/gaia/commit/af7447ccc38559868a2bd1dcfa03b4444ef5591e

Comment 11

[Tony Chung [:tchung]]

Jsmith, mind taking a moztrap stab at this?  if not, we can discuss chunking up on tests since it affects multiple apps.

From Djf's email

The following apps now have the storage permission:

$ grep -l '"storage"' *apps/*/manifest.webapp
apps/camera/manifest.webapp
apps/clock/manifest.webapp
apps/communications/manifest.webapp
apps/costcontrol/manifest.webapp
apps/email/manifest.webapp
apps/fm/manifest.webapp
apps/gallery/manifest.webapp
apps/music/manifest.webapp
apps/settings/manifest.webapp
apps/sms/manifest.webapp
apps/system/manifest.webapp
apps/video/manifest.webapp
test_apps/test-container/manifest.webapp
test_external_apps/mochitest/manifest.webapp

Apps using indexedDB:

$ grep -l indexedDB *apps/*/js/*.js
apps/browser/js/places.js
apps/calendar/js/db.js
apps/calendar/js/mozalarm_shim.js
apps/camera/js/camera.js
apps/clock/js/alarm.js
apps/clock/js/alarmsdb.js
apps/homescreen/js/state.js
apps/system/js/window_manager.js
test_apps/image-uploader/js/credentials_db.js

Comment 12

[Jason Smith [:jsmith]]

Yeah, i can look into getting tests here.

Comment 13

[David Flanagan [:djf]]

I think it is going to be very hard to write tests for this.  Gallery stores image thumbnails in the db. At a couple of kb per thumbnail, it would take 2 or 3000 pictures on the phone to reach the 5mb threshold. But there really isn't anyway to be sure that the db has crossed that threshold, so you couldn't even be sure that the test was actually testing the > 5mb condition you need to test.

Apps like clock and camera use the database only incidentally and will probably never exceed the threshold.  The permission was added here just to be safe.

Comment 14

[Jeni]

Can you please provide steps to verify this fix - as we will blackbox test from the UI?

Comment 15

[Jason Smith [:jsmith]]

(In reply to David Flanagan [:djf] from comment #13)
> I think it is going to be very hard to write tests for this.  Gallery stores
> image thumbnails in the db. At a couple of kb per thumbnail, it would take 2
> or 3000 pictures on the phone to reach the 5mb threshold. But there really
> isn't anyway to be sure that the db has crossed that threshold, so you
> couldn't even be sure that the test was actually testing the > 5mb condition
> you need to test.
> 
> Apps like clock and camera use the database only incidentally and will
> probably never exceed the threshold.  The permission was added here just to
> be safe.

I agree. What I think is the critical piece to test isn't here specifically with the gaia apps specifically, it's the underlying gecko pieces. I'll look into doing that on bug 856032 instead - that will allow me to do the necessary testing around this.

Comment 16

[cbarker]

David Flanagan if you can Verify this bug out, it would be much appreciated.

Comment 17

[Jason Smith [:jsmith]]

(In reply to cbarker from comment #16)
> David Flanagan if you can Verify this bug out, it would be much appreciated.

This a code change that does not need end-user verification.

Comment 18

[amiller]

Verified as fixed per comment 17.