[binding] Migrate XSLTProcessor.idl away from using custom V8 binding.

[binding] Migrate XSLTProcessor.idl away from using custom V8 binding.

The CL removes the custom V8 bindings required for XSLTProcessor.idl.

R=haraken@chromium.org

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

[vivekg, 2014-12-11 12:19:41]

vivekg@chromium.org changed reviewers:
+ vivekg@chromium.org

[vivekg, 2014-12-11 12:19:41]

PTAL, thanks!

[Jens Widell, 2014-12-11 13:26:14]

jl@opera.com changed reviewers:
+ jl@opera.com

[Jens Widell, 2014-12-11 13:26:14]

https://codereview.chromium.org/795053002/diff/1/Source/core/xml/XSLTProcesso...
File Source/core/xml/XSLTProcessor.idl (right):

https://codereview.chromium.org/795053002/diff/1/Source/core/xml/XSLTProcesso...
Source/core/xml/XSLTProcessor.idl:45: void setParameter(DOMString namespaceURI,
[TreatNullAs=NullString, TreatUndefinedAs=NullString] DOMString localName,
[TreatNullAs=NullString, TreatUndefinedAs=NullString] DOMString value);
[TreatNullAs=NullString] is the same as making the type "DOMString?" (i.e.
nullable). We prefer the latter, since it's shorter and isn't a Blink IDL
extension.

Have you checked how Firefox or IE handles null and undefined for these
arguments? Silently rejecting null/undefined is a bit unusual. Maybe we can just
make all the arguments plain DOMString, meaning null and undefined get converted
to the strings "null" and "undefined"?

[vivekg, 2014-12-11 15:28:33]

Below are the table summarizing the behavior differences.



FF: setParameter
================
+-----------+--------------+--------------------------------------------------------+
|    key    |    value     |      result of getParameter(key)                   
   |
+-----------+--------------+--------------------------------------------------------+
| 'testKey' | 'testValue'  | 'testValue'                                        
   |
+-----------+--------------+--------------------------------------------------------+
| 'testKey' | undefined    | NS_ERROR_FAILURE: Component returned               
   |
|           |              | failure code: 0x80004005                           
   |
|           |              | (NS_ERROR_FAILURE) [nsIXSLTProcessor.setParameter] 
   |
+-----------+--------------+--------------------------------------------------------+
| 'testKey' | null         | NS_ERROR_FAILURE: Component returned               
   |
|           |              | failure code: 0x80004005                           
   |
|           |              | (NS_ERROR_FAILURE) [nsIXSLTProcessor.setParameter] 
   |
+-----------+--------------+--------------------------------------------------------+
| undefined | 'testValue'  | 'testValue'                                        
   |
+-----------+--------------+--------------------------------------------------------+
| null      | 'testValue'  | 'testValue'                                        
   |
+-----------+--------------+--------------------------------------------------------+



IE-9 (9.0.8112.16421) and IE-11 (11.0.9600.16428): setParameter
================================================================
+-----------+--------------+--------------------------------------------------------+
|    key    |    value     |      result of getParameter(key)                   
   |
+-----------+--------------+--------------------------------------------------------+
| 'testKey' | 'testValue'  | 'testValue'                                        
   |
+-----------+--------------+--------------------------------------------------------+
| 'testKey' | undefined    | ''                                                 
   |
+-----------+--------------+--------------------------------------------------------+
| 'testKey' | null         | ''                                                 
   |
+-----------+--------------+--------------------------------------------------------+
| undefined | 'testValue'  | Error: An empty string '' is not a valid name.     
   |
+-----------+--------------+--------------------------------------------------------+
| null      | 'testValue'  | Error: Type mismatch                               
   |
+-----------+--------------+--------------------------------------------------------+



Opera (12.17 presto based): setParameter
========================================
+-----------+--------------+--------------------------------------------------------+
|    key    |    value     |      result of getParameter(key)                   
   |
+-----------+--------------+--------------------------------------------------------+
| 'testKey' | 'testValue'  | 'testValue'                                        
   |
+-----------+--------------+--------------------------------------------------------+
| 'testKey' | undefined    | undefined                                          
   |
+-----------+--------------+--------------------------------------------------------+
| 'testKey' | null         | null                                               
   |
+-----------+--------------+--------------------------------------------------------+
| undefined | 'testValue'  | 'testValue'                                        
   |
+-----------+--------------+--------------------------------------------------------+
| null      | 'testValue'  | 'testValue'                                        
   |
+-----------+--------------+--------------------------------------------------------+



Latest Chrome (41.0.2246.0): setParameter
=========================================
+-----------+--------------+--------------------------------------------------------+
|    key    |    value     |      result of getParameter(key)                   
   |
+-----------+--------------+--------------------------------------------------------+
| 'testKey' | 'testValue'  | 'testValue'                                        
   |
+-----------+--------------+--------------------------------------------------------+
| 'testKey' | undefined    | undefined                                          
   |
+-----------+--------------+--------------------------------------------------------+
| 'testKey' | null         | undefined                                          
   |
+-----------+--------------+--------------------------------------------------------+
| undefined | 'testValue'  | undefined                                          
   |
+-----------+--------------+--------------------------------------------------------+
| null      | 'testValue'  | undefined                                          
   |
+-----------+--------------+--------------------------------------------------------+

[vivekg, 2014-12-11 15:31:26]

FF: setParameter
================
+-----------+--------------+---------------------------------------------+
|    key    |    value     |      result of getParameter(key)            |
+-----------+--------------+---------------------------------------------+
| 'testKey' | 'testValue'  | 'testValue'                                 |
+-----------+--------------+---------------------------------------------+
| 'testKey' | undefined    | NS_ERROR_FAILURE: Component returned        |
|           |              | failure code: 0x80004005(NS_ERROR_FAILURE)  |
|           |              | [nsIXSLTProcessor.setParameter]             |
+-----------+--------------+---------------------------------------------+
| 'testKey' | null         | NS_ERROR_FAILURE: Component returned        |
|           |              | failure code: 0x80004005(NS_ERROR_FAILURE)  |
|           |              | [nsIXSLTProcessor.setParameter]             |
+-----------+--------------+---------------------------------------------+
| undefined | 'testValue'  | 'testValue'                                 |
+-----------+--------------+---------------------------------------------+
| null      | 'testValue'  | 'testValue'                                 |
+-----------+--------------+---------------------------------------------+



IE-9 (9.0.8112.16421) and IE-11 (11.0.9600.16428): setParameter
================================================================
+-----------+--------------+---------------------------------------------+
|    key    |    value     |      result of getParameter(key)            |
+-----------+--------------+---------------------------------------------+
| 'testKey' | 'testValue'  | 'testValue'                                 |
+-----------+--------------+---------------------------------------------+
| 'testKey' | undefined    | ''                                          |
+-----------+--------------+---------------------------------------------+
| 'testKey' | null         | ''                                          |
+-----------+--------------+---------------------------------------------+
| undefined | 'testValue'  | An empty string '' is not a valid name.     |
+-----------+--------------+---------------------------------------------+
| null      | 'testValue'  | Type mismatch                               |
+-----------+--------------+---------------------------------------------+



Opera (12.17 presto based): setParameter
========================================
+-----------+--------------+---------------------------------------------+
|    key    |    value     |      result of getParameter(key)            |
+-----------+--------------+---------------------------------------------+
| 'testKey' | 'testValue'  | 'testValue'                                 |
+-----------+--------------+---------------------------------------------+
| 'testKey' | undefined    | undefined                                   |
+-----------+--------------+---------------------------------------------+
| 'testKey' | null         | null                                        |
+-----------+--------------+---------------------------------------------+
| undefined | 'testValue'  | 'testValue'                                 |
+-----------+--------------+---------------------------------------------+
| null      | 'testValue'  | 'testValue'                                 |
+-----------+--------------+---------------------------------------------+



Latest Chrome (41.0.2246.0): setParameter
=========================================
+-----------+--------------+---------------------------------------------+
|    key    |    value     |      result of getParameter(key)            |
+-----------+--------------+---------------------------------------------+
| 'testKey' | 'testValue'  | 'testValue'                                 |
+-----------+--------------+---------------------------------------------+
| 'testKey' | undefined    | undefined                                   |
+-----------+--------------+---------------------------------------------+
| 'testKey' | null         | undefined                                   |
+-----------+--------------+---------------------------------------------+
| undefined | 'testValue'  | undefined                                   |
+-----------+--------------+---------------------------------------------+
| null      | 'testValue'  | undefined                                   |
+-----------+--------------+---------------------------------------------+

[vivekg, 2014-12-11 15:40:28]

> 
> Latest Chrome (41.0.2246.0): setParameter
> =========================================
> +-----------+--------------+---------------------------------------------+
> |    key    |    value     |      result of getParameter(key)            |
> +-----------+--------------+---------------------------------------------+
> | 'testKey' | 'testValue'  | 'testValue'                                 |
> +-----------+--------------+---------------------------------------------+
> | 'testKey' | undefined    | undefined                                   |
> +-----------+--------------+---------------------------------------------+
> | 'testKey' | null         | undefined                                   |
> +-----------+--------------+---------------------------------------------+
> | undefined | 'testValue'  | undefined                                   |
> +-----------+--------------+---------------------------------------------+
> | null      | 'testValue'  | undefined                                   |
> +-----------+--------------+---------------------------------------------+

With the usage of "DOMString?", I am getting the table as below as against
above.

+-----------+--------------+---------------------------------------------+
|    key    |    value     |      result of getParameter(key)            |
+-----------+--------------+---------------------------------------------+
| undefined | 'testValue'  | 'testValue'                                 |
+-----------+--------------+---------------------------------------------+

[Jens Widell, 2014-12-11 16:02:24]

Thanks for detailed investigation!

My interpretation of those results is that there isn't much interoperability to
worry about making worse, regardless of what we do, so I think we should just do
something that makes some kind of sense and is simple.

I'd make these plain DOMString arguments and remove all special handling of null
and undefined (i.e. let all input values, including null and undefined, be
converted to strings and used as such.) Except for the namespaceURI argument,
which would make sense to make nullable ("DOMString?").

How quaint of you to test with Presto, BTW. :-) Since I wrote that
implementation of XSLTProcessor, I can comment on what it actually does. In
terms of IDL (in reality it's all hand-written C++) the setParameter() method
behaves as if it was defined:

  void setParameter(DOMString? namespaceURI, DOMString localName, any value);

Except Presto's implementation of "nullable" treats undefined as if it was null,
instead of converting it to the string "undefined".

[vivekg, 2014-12-12 05:00:19]

On 2014/12/11 at 16:02:24, jl wrote:
> I'd make these plain DOMString arguments and remove all special handling of
null and undefined (i.e. let all input values, including null and undefined, be
converted to strings and used as such.) Except for the namespaceURI argument,
which would make sense to make nullable ("DOMString?").
> 

Sure, makes sense.


> How quaint of you to test with Presto, BTW. :-) Since I wrote that
implementation of XSLTProcessor, I can comment on what it actually does. In
terms of IDL (in reality it's all hand-written C++) the setParameter() method
behaves as if it was defined:
> 

Thank you. I have the windows machine installed with presto based opera.
So when you asked about IE and FF behavior, I thought of checking it even on
opera. :)

>   void setParameter(DOMString? namespaceURI, DOMString localName, any value);
> 
> Except Presto's implementation of "nullable" treats undefined as if it was
null, instead of converting it to the string "undefined".

Done.


New patch uploaded, PTAL. Thanks.

[Jens Widell, 2014-12-12 06:50:53]

LGTM

Note that I'm not core/ OWNER.

[haraken, 2014-12-12 07:09:00]

LGTM

[vivekg, 2014-12-12 07:11:51]

The CQ bit was checked by vivekg@chromium.org

[commit-bot: I haz the power, 2014-12-12 07:12:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/795053002/20001

[commit-bot: I haz the power, 2014-12-12 07:25:54]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=187003