Deprecate FileError in FileAPI

Deprecate FileError in FileAPI

Apps should expect DOMError instead.

This change breaks existing apps, so I kept FileError in this CL
but made it inherit from DOMError and marked '.code' attribute
deprecated.

BUG=276861
TEST=fast/files/*,fast/filesystem/*

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=156562

[kinuko, 2013-08-21 12:10:06]

Hi, recently I was just pinged that we should use DOMError instead of FileError
in FileAPI code. Can you review? (Most changes in LayoutTests are mechanical)

[kinuko, 2013-08-21 12:26:40]

On 2013/08/21 12:10:06, kinuko wrote:
> Hi, recently I was just pinged that we should use DOMError instead of
FileError
> in FileAPI code. Can you review? (Most changes in LayoutTests are mechanical)

D'oh, I was then told that DOMError might be nuked in favor of DOMException too.
Hmm, probably I should withdraw this change for now? :(

[Mike West, 2013-08-21 12:43:59]

+arv.

I have no idea. Erik?

https://codereview.chromium.org/22831019/diff/5001/Source/core/page/UseCounte...
File Source/core/page/UseCounter.cpp (right):

https://codereview.chromium.org/22831019/diff/5001/Source/core/page/UseCounte...
Source/core/page/UseCounter.cpp:665: return "FileError is deprecated. Refer
'name' or 'message' attribute of DOMError instead of 'code'.";
Nit: "Please use the 'name' or 'message' attributes of DOMError rather than
'code'."

[arv (Not doing code reviews), 2013-08-21 14:15:21]

https://codereview.chromium.org/22831019/diff/5001/Source/core/fileapi/FileEr...
File Source/core/fileapi/FileError.idl (right):

https://codereview.chromium.org/22831019/diff/5001/Source/core/fileapi/FileEr...
Source/core/fileapi/FileError.idl:32: ] interface FileError : DOMError {
Maybe [NoInterfaceObject] as well?

[do-not-use, 2013-08-21 14:19:04]

https://codereview.chromium.org/22831019/diff/5001/Source/core/fileapi/FileEr...
File Source/core/fileapi/FileError.idl (right):

https://codereview.chromium.org/22831019/diff/5001/Source/core/fileapi/FileEr...
Source/core/fileapi/FileError.idl:32: ] interface FileError : DOMError {
On 2013/08/21 14:15:22, arv wrote:
> Maybe [NoInterfaceObject] as well?

Using [NoInterfaceObject] may break existing applications as
FileError.NOT_FOUND_ERR (and other constants) will not longer be accessible.
Constants will only be accessible on FileError instances.

[arv (Not doing code reviews), 2013-08-21 14:21:38]

https://codereview.chromium.org/22831019/diff/5001/Source/core/fileapi/FileEr...
File Source/core/fileapi/FileError.idl (right):

https://codereview.chromium.org/22831019/diff/5001/Source/core/fileapi/FileEr...
Source/core/fileapi/FileError.idl:32: ] interface FileError : DOMError {
On 2013/08/21 14:19:05, Christophe Dumez wrote:
> On 2013/08/21 14:15:22, arv wrote:
> > Maybe [NoInterfaceObject] as well?
> 
> Using [NoInterfaceObject] may break existing applications as
> FileError.NOT_FOUND_ERR (and other constants) will not longer be accessible.
> Constants will only be accessible on FileError instances.

Good point.

My goal was to get people of off FileError. Maybe add a DeprecateAs to the
interface (I'm not even sure that works)?

[do-not-use, 2013-08-21 14:40:01]

On 2013/08/21 14:21:38, arv wrote:
>
https://codereview.chromium.org/22831019/diff/5001/Source/core/fileapi/FileEr...
> File Source/core/fileapi/FileError.idl (right):
> 
>
https://codereview.chromium.org/22831019/diff/5001/Source/core/fileapi/FileEr...
> Source/core/fileapi/FileError.idl:32: ] interface FileError : DOMError {
> On 2013/08/21 14:19:05, Christophe Dumez wrote:
> > On 2013/08/21 14:15:22, arv wrote:
> > > Maybe [NoInterfaceObject] as well?
> > 
> > Using [NoInterfaceObject] may break existing applications as
> > FileError.NOT_FOUND_ERR (and other constants) will not longer be accessible.
> > Constants will only be accessible on FileError instances.
> 
> Good point.
> 
> My goal was to get people of off FileError. Maybe add a DeprecateAs to the
> interface (I'm not even sure that works)?

Good idea, I believe we can use [DeprecateAs] on interfaces with this minor
change:
http://pastebin.com/rpCWvnbd

This will basically forward the [DeprecatedAs] extended attribute from the
interface to its Constructor attribute on the global Window object. Calling
FileError.NOT_FOUND_ERR should then print the deprecation warning as expected.

[kinuko, 2013-08-21 16:27:45]

Thanks Erik, Christophe and Mike!  I went on updating the patch.

> Good idea, I believe we can use [DeprecateAs] on interfaces with this minor
change:
> http://pastebin.com/rpCWvnbd

I've tried this, but it looks like this works only for constructors, while
accessing FileErorr's constants doesn't require / go through its constructor
code.  Probably we could add more code in the code generator to make it work for
constants, but I'm not sure if it's easy or not.  If we want to do so I can look
into it tomorrow again.

https://codereview.chromium.org/22831019/diff/5001/Source/core/page/UseCounte...
File Source/core/page/UseCounter.cpp (right):

https://codereview.chromium.org/22831019/diff/5001/Source/core/page/UseCounte...
Source/core/page/UseCounter.cpp:665: return "FileError is deprecated. Refer
'name' or 'message' attribute of DOMError instead of 'code'.";
On 2013/08/21 12:44:00, Mike West wrote:
> Nit: "Please use the 'name' or 'message' attributes of DOMError rather than
> 'code'."

Done.

[do-not-use, 2013-08-21 16:45:07]

On 2013/08/21 16:27:45, kinuko wrote:
> Thanks Erik, Christophe and Mike!  I went on updating the patch.
> 
> > Good idea, I believe we can use [DeprecateAs] on interfaces with this minor
> change:
> > http://pastebin.com/rpCWvnbd

Right, sorry about that.

I had a look and DOMWindow constructor attributes behave differently than other
attributes and they don't support [DeprecateAs] yet.
Considering more work is needed, I think it is fine not to include it in this
CL.

[arv (Not doing code reviews), 2013-08-21 16:50:19]

LGTM

[do-not-use, 2013-08-21 16:53:50]

LGTM2

[commit-bot: I haz the power, 2013-08-22 02:49:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kinuko@chromium.org/22831019/4001

[commit-bot: I haz the power, 2013-08-22 05:28:54]

Retried try job too often on win_blink_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_blink_...

[commit-bot: I haz the power, 2013-08-22 14:14:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kinuko@chromium.org/22831019/28001

[commit-bot: I haz the power, 2013-08-22 16:03:33]

Change committed as 156562