1299500 - Get rid of DeviceStorage API

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Comment 1

[Andrea Marchesini [:baku]]

Attachment: part 1 - DeviceStorageAreaChangedEvent

Comment 2

[Andrea Marchesini [:baku]]

Attachment: part 2 - DeviceStorageChangeEvent

Comment 3

[Andrea Marchesini [:baku]]

Attachment: part 3 - DeviceStorageAreaListener.

Comment 4

[Andrea Marchesini [:baku]]

Attachment: part 4 - Navigator

Comment 5

[Andrea Marchesini [:baku]]

Attachment: part 5 - Directory::CreateFile

Comment 6

[Andrea Marchesini [:baku]]

Attachment: part 6 - Directory::CreateDirectory

Comment 7

[Andrea Marchesini [:baku]]

Attachment: part 7 - Directory::Remove

Comment 8

[Andrea Marchesini [:baku]]

Attachment: part 8 - Directory::Get

This actor is needed for Entries API. I moved some code here without removing the actor.

Comment 9

[Andrea Marchesini [:baku]]

Attachment: part 9 - Directory::GetRoot

Comment 10

[Andrea Marchesini [:baku]]

Attachment: part 10 - DeviceStorage

I haven't found a nice way to split this patch.

Comment 11

[Andrea Marchesini [:baku]]

Attachment: part 11 - FileSystem

Comment 12

[Andrea Marchesini [:baku]]

Note that I didn't touch dom/system/gonk. This code now is broken, but probably it's already broken: it's not compiled in our treeherder builds and it's not used anymore (am I wrong?).

Comment 13

[(no longer active)]

(In reply to Andrea Marchesini [:baku] from comment #12)
> Note that I didn't touch dom/system/gonk. This code now is broken, but
> probably it's already broken: it's not compiled in our treeherder builds and
> it's not used anymore (am I wrong?).

You're right, dom/system/gonk isn't built.  I think we're not supposed to remove gonk code (yet?)

Comment 14

[(no longer active)]

Comment on attachment 8844484 [details] [diff] [review]
part 6 - Directory::CreateDirectory

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

::: dom/filesystem/Directory.h
@@ -18,5 @@
> -// Windows header files. Undefine the macro of CreateDirectory to avoid
> -// Directory#CreateDirectory being replaced by Directory#CreateDirectoryW.
> -#ifdef CreateDirectory
> -#undef CreateDirectory
> -#endif

Be careful removing this stuff... Make sure to get Windows try runs.  ;-)

Comment 15

[(no longer active)]

I'll look at the last part tomorrow.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/92606f748fcb
Get rid of DeviceStorage API - part 1 - DeviceStorageAreaChangedEvent, r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/41cbf92cfad2
Get rid of DeviceStorage API - part 2 - DeviceStorageChangeEvent, r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/07ed303b1e3e
Get rid of DeviceStorage API - part 3 - DeviceStorageAreaListener, r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/a35a754a55d8
Get rid of DeviceStorage API - part 4 - Navigator, r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8670bee71a5
Get rid of DeviceStorage API - part 5 - Directory::CreateFile, r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/f6e90b7c442f
Get rid of DeviceStorage API - part 6 - Directory::CreateDirectory, r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/f28b200fd93d
Get rid of DeviceStorage API - part 7 - Directory::Remove, r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/f4c12243de18
Get rid of DeviceStorage API - part 8 - Directory::Get, r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/3364716e5765
Get rid of DeviceStorage API - part 9 - Directory::GetRoot, r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/6d0ea5812180
Get rid of DeviceStorage API - part 10 - DeviceStorage, r=ehsan, r=billm
https://hg.mozilla.org/integration/mozilla-inbound/rev/c539b2e9f9c9
Get rid of DeviceStorage API - part 11 - FileSystem, r=ehsan

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/92606f748fcb
https://hg.mozilla.org/mozilla-central/rev/41cbf92cfad2
https://hg.mozilla.org/mozilla-central/rev/07ed303b1e3e
https://hg.mozilla.org/mozilla-central/rev/a35a754a55d8
https://hg.mozilla.org/mozilla-central/rev/a8670bee71a5
https://hg.mozilla.org/mozilla-central/rev/f6e90b7c442f
https://hg.mozilla.org/mozilla-central/rev/f28b200fd93d
https://hg.mozilla.org/mozilla-central/rev/f4c12243de18
https://hg.mozilla.org/mozilla-central/rev/3364716e5765
https://hg.mozilla.org/mozilla-central/rev/6d0ea5812180
https://hg.mozilla.org/mozilla-central/rev/c539b2e9f9c9

Comment 18

[Andrea Marchesini [:baku]]

Bug 1344415 requires this bug to be uplifted.

Comment 19

[Andrea Marchesini [:baku]]

Comment on attachment 8844471 [details] [diff] [review]
part 1 - DeviceStorageAreaChangedEvent

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1332003

See comment https://bugzilla.mozilla.org/show_bug.cgi?id=1344415#c39 for more information about why uplift these patches.

Comment 20

[Gerry Chang [:gchang]]

Comment on attachment 8844471 [details] [diff] [review]
part 1 - DeviceStorageAreaChangedEvent

Required fix for bug 1344415. Aurora54+.

Comment 21

[Carsten Book [:Tomcat]]

at least part 10 needs rebasing like

warning: conflicts while merging toolkit/components/jsdownloads/src/DownloadIntegration.jsm! (edit, then use 'hg resolve --mark')
warning: conflicts while merging toolkit/components/jsdownloads/test/unit/common_test_Download.js! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')

Comment 22

[Andrea Marchesini [:baku]]

Attachment: 3364716e5765 for m-a

Comment 23

[Andrea Marchesini [:baku]]

Attachment: 6d0ea5812180 for m-a

Comment 24

[Carsten Book [:Tomcat]]

need l10n approval too 

remote: ************************** ERROR ****************************
remote: 
remote: * File used for localization (mobile/android/locales/en-US/chrome/browser.properties) altered in this changeset *
remote: 
remote: This repository is string frozen. Please request explicit permission from
remote: release managers to break string freeze in your bug.
remote: If you have that explicit permission, denote that by including in
remote: your commit message l10n=...
remote: *************************************************************

Comment 25

[Carsten Book [:Tomcat]]

seems thats from part 10

Comment 26

[Francesco Lodolo [:flod]]

Can we get a part 10 without the string changes only for aurora? Literally same patch, without the changes to mobile/android/locales/en-US/chrome/browser.properties

Patch is just removing string, but it's going to create a chain effect on tools and a lot of noise mid-cycle. If we can avoid that, it would be great.

Comment 27

[Andrea Marchesini [:baku]]

Attachment: 6d0ea5812180 for m-a

Comment 28

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f53f7fbff21d
https://hg.mozilla.org/releases/mozilla-aurora/rev/e2612175b358
https://hg.mozilla.org/releases/mozilla-aurora/rev/06895382788a
https://hg.mozilla.org/releases/mozilla-aurora/rev/7a8b6f80ae34
https://hg.mozilla.org/releases/mozilla-aurora/rev/91894537e3d2
https://hg.mozilla.org/releases/mozilla-aurora/rev/024b93a93726
https://hg.mozilla.org/releases/mozilla-aurora/rev/a74303464cee
https://hg.mozilla.org/releases/mozilla-aurora/rev/52a3a2c64671
https://hg.mozilla.org/releases/mozilla-aurora/rev/1961741e8e7d
https://hg.mozilla.org/releases/mozilla-aurora/rev/7c8fddc43c02
https://hg.mozilla.org/releases/mozilla-aurora/rev/e4c10d4f6b3e

Comment 29

[Carsten Book [:Tomcat]]

Required fix for bug 1344415. Aurora54+. -- baku, gchang since this other bug has also more uplift approvals for esr52 and beta we need this one here too right ? Baku if so, can fill out the requests, thanks :)

Comment 30

[Andrea Marchesini [:baku]]

Comment on attachment 8844490 [details] [diff] [review]
part 11 - FileSystem

[Approval Request Comment]
User impact if declined: These patches are needed for a security fix: Bug 1344415
Fix Landed on Version: aurora and nightly.
Risk to taking this patch (and alternatives if risky): not too much. This API is not exposed to content but just to b2g/connected devices.
String or UUID changes made by this patch:  none


Approval Request Comment
[Feature/Bug causing the regression]: Bug 1332003
[User impact if declined]: This is needed as dependence. No impact to users
[Is this code covered by automated tests?]: it removes code.
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: n/a 
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: none
[Why is the change risky/not risky?]: it removes an already-disabled API.
[String changes made/needed]: none

Comment 31

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8844490 [details] [diff] [review]
part 11 - FileSystem

If this was only used by b2g, should be ok. A bit scary to take this for beta though.

Comment 32

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/188a791aec8b
https://hg.mozilla.org/releases/mozilla-beta/rev/71d55c228948
https://hg.mozilla.org/releases/mozilla-beta/rev/c51eb9dcfded
https://hg.mozilla.org/releases/mozilla-beta/rev/0a2cc0c61dcb
https://hg.mozilla.org/releases/mozilla-beta/rev/e2684afbe33b
https://hg.mozilla.org/releases/mozilla-beta/rev/4b38e9c76323
https://hg.mozilla.org/releases/mozilla-beta/rev/fd799307edc0
https://hg.mozilla.org/releases/mozilla-beta/rev/351b387d389f
https://hg.mozilla.org/releases/mozilla-beta/rev/351b387d389f
https://hg.mozilla.org/releases/mozilla-beta/rev/6db8154a2542
https://hg.mozilla.org/releases/mozilla-beta/rev/4816ac7ebaf4
https://hg.mozilla.org/releases/mozilla-beta/rev/57035f3bfd55

Comment 33

[Julien Cristau [:jcristau]]

Comment on attachment 8844471 [details] [diff] [review]
part 1 - DeviceStorageAreaChangedEvent

remove b2g-only api, needed for bug 1344415's fix, esr52+

Comment 34

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/5fdd36b4400a
https://hg.mozilla.org/releases/mozilla-esr52/rev/34776df5ce44
https://hg.mozilla.org/releases/mozilla-esr52/rev/26189af0f504
https://hg.mozilla.org/releases/mozilla-esr52/rev/798a8fe17e7b
https://hg.mozilla.org/releases/mozilla-esr52/rev/daf2e4f2bd5c
https://hg.mozilla.org/releases/mozilla-esr52/rev/1187091c3134
https://hg.mozilla.org/releases/mozilla-esr52/rev/a908f2c2fe30
https://hg.mozilla.org/releases/mozilla-esr52/rev/e95a26cf7a42
https://hg.mozilla.org/releases/mozilla-esr52/rev/d63f3b14e571
https://hg.mozilla.org/releases/mozilla-esr52/rev/08f2bc167ae8
https://hg.mozilla.org/releases/mozilla-esr52/rev/263f27805689

Comment 35

[Frank-Rainer Grahl (:frg)]

With the patch in esr52 I am having problems compiling SeaMonkey but I think Thunderbird and Firefox are also affected. Just didn't try yet. I am getting C2061 errors in various comm-esr52 and mozilla-esr52 files which include FileSystemDirectoryReader.h. Always the same error. comm-beta compiles fine so I think the patch needs adjustment.

+++ snip +++

5 -we4553 -GR-  -O2 -Oy   -Fdgenerated.pdb  d:/seabuild/release/comm-esr52-15/obj-x86_64-pc-mingw32/dom/bindings/UnifiedBindings5.cpp
UnifiedBindings5.cpp
d:/seabuild/release/comm-esr52-15/obj-x86_64-pc-mingw32/dist/include\mozilla/dom/FileSystemDirectoryReader.h(31): error C2061: syntax error: identifier 'FileSystemDirectoryEntry'
d:/seabuild/release/comm-esr52-15/obj-x86_64-pc-mingw32/dist/include\mozilla/dom/FileSystemDirectoryReader.h(53): error C2065: 'FileSystemDirectoryEntry': undeclared identifier
d:/seabuild/release/comm-esr52-15/obj-x86_64-pc-mingw32/dist/include\mozilla/dom/FileSystemDirectoryReader.h(53): error C2923: 'RefPtr': 'FileSystemDirectoryEntry' is not a valid template type argument for parameter 'T'
d:/seabuild/release/comm-esr52-15/obj-x86_64-pc-mingw32/dist/include\mozilla/dom/FileSystemDirectoryReader.h(38): error C2678: binary '->': no operator found which takes a left-hand operand of type 'const RefPtr' (or there is no acceptable conversion)
d:\seabuild\release\comm-esr52-15\obj-x86_64-pc-mingw32\dist\include\mozilla/RefPtr.h(316): note: could be 'T *RefPtr<T>::operator ->(void) const'
d:/seabuild/release/comm-esr52-15/obj-x86_64-pc-mingw32/dist/include\mozilla/dom/FileSystemDirectoryReader.h(38): note: while trying to match the argument list '(const RefPtr)'
d:/seabuild/release/comm-esr52-15/obj-x86_64-pc-mingw32/dist/include\mozilla/dom/FileSystemDirectoryReader.h(38): error C2039: 'GetParentObject': is not a member of 'RefPtr'
d:\seabuild\release\comm-esr52-15\obj-x86_64-pc-mingw32\dist\include\mozilla/RefPtr.h(48): note: see declaration of 'RefPtr'

Comment 36

[Tom Ritter [:tjr]]

(In reply to Frank-Rainer Grahl from comment #35)
> With the patch in esr52 I am having problems compiling SeaMonkey but I think
> Thunderbird and Firefox are also affected. Just didn't try yet. I am getting
> C2061 errors in various comm-esr52 and mozilla-esr52 files which include
> FileSystemDirectoryReader.h. Always the same error. comm-beta compiles fine
> so I think the patch needs adjustment.
> 
> +++ snip +++
> 
> 5 -we4553 -GR-  -O2 -Oy   -Fdgenerated.pdb 
> d:/seabuild/release/comm-esr52-15/obj-x86_64-pc-mingw32/dom/bindings/
> UnifiedBindings5.cpp
> UnifiedBindings5.cpp
> d:/seabuild/release/comm-esr52-15/obj-x86_64-pc-mingw32/dist/include\mozilla/
> dom/FileSystemDirectoryReader.h(31): error C2061: syntax error: identifier
> 'FileSystemDirectoryEntry'
> d:/seabuild/release/comm-esr52-15/obj-x86_64-pc-mingw32/dist/include\mozilla/
> dom/FileSystemDirectoryReader.h(53): error C2065:
> 'FileSystemDirectoryEntry': undeclared identifier
> d:/seabuild/release/comm-esr52-15/obj-x86_64-pc-mingw32/dist/include\mozilla/
> dom/FileSystemDirectoryReader.h(53): error C2923: 'RefPtr':
> 'FileSystemDirectoryEntry' is not a valid template type argument for
> parameter 'T'
> d:/seabuild/release/comm-esr52-15/obj-x86_64-pc-mingw32/dist/include\mozilla/
> dom/FileSystemDirectoryReader.h(38): error C2678: binary '->': no operator
> found which takes a left-hand operand of type 'const RefPtr' (or there is no
> acceptable conversion)
> d:\seabuild\release\comm-esr52-15\obj-x86_64-pc-mingw32\dist\include\mozilla/
> RefPtr.h(316): note: could be 'T *RefPtr<T>::operator ->(void) const'
> d:/seabuild/release/comm-esr52-15/obj-x86_64-pc-mingw32/dist/include\mozilla/
> dom/FileSystemDirectoryReader.h(38): note: while trying to match the
> argument list '(const RefPtr)'
> d:/seabuild/release/comm-esr52-15/obj-x86_64-pc-mingw32/dist/include\mozilla/
> dom/FileSystemDirectoryReader.h(38): error C2039: 'GetParentObject': is not
> a member of 'RefPtr'
> d:\seabuild\release\comm-esr52-15\obj-x86_64-pc-mingw32\dist\include\mozilla/
> RefPtr.h(48): note: see declaration of 'RefPtr'

I opened https://bugzilla.mozilla.org/show_bug.cgi?id=1353765 for this

Comment 37

[Andrei Vaida [:avaida]]

Setting qe-verify- based on Andrea's assessment on manual testing needs (see Comment 30).

Comment 38

[Chris Mills [:cmills]]

I've checked MDN, and all of the related API features either haven't been documented at all, or have had their pages moved to the B2G OS archive.

I've added a note to the Fx55 release notes:

https://developer.mozilla.org/en-US/Firefox/Releases/55#Removals_from_the_web_platform