canvas.getContext with alpha:undefined should behave like alpha:true

canvas.getContext with alpha:undefined should behave like alpha:true

WebIDL dictionary spec says that setting a dictionary value to null or
undefined should result in the attribute retaining its default value.
So, if alpha is undefined or null, it should be converted as true.

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

[zino, 2014-05-18 05:09:29]

Please take a look.

Thank you.

[Justin Novosad, 2014-05-20 19:28:39]

https://codereview.chromium.org/290143007/diff/20001/Source/bindings/v8/custo...
File Source/bindings/v8/custom/V8HTMLCanvasElementCustom.cpp (right):

https://codereview.chromium.org/290143007/diff/20001/Source/bindings/v8/custo...
Source/bindings/v8/custom/V8HTMLCanvasElementCustom.cpp:50: static bool
determineAlpha(v8::Handle<v8::Value> alphaValue)
We are going to need a more generic version of this function to handle all the
other attributes that suffer from the same bug with WebGL.  Suggestion:
template<T> static T readDictionaryValue(v8::Handle<v8::Value>, const T&
default)

https://codereview.chromium.org/290143007/diff/20001/Source/bindings/v8/custo...
Source/bindings/v8/custom/V8HTMLCanvasElementCustom.cpp:74:
webGLAttributes->setAlpha(determineAlpha(jsAttributes->Get(alpha)));
Theroriginal bug was primarily about 2D canvas. If you are going to fix the
webGL case as well, you should add a test for it, and you should also fix the
other attributes that have the same problem.

[zino, 2014-05-21 10:29:50]

On 2014/05/20 19:28:39, junov wrote:
>
https://codereview.chromium.org/290143007/diff/20001/Source/bindings/v8/custo...
> File Source/bindings/v8/custom/V8HTMLCanvasElementCustom.cpp (right):
> 
>
https://codereview.chromium.org/290143007/diff/20001/Source/bindings/v8/custo...
> Source/bindings/v8/custom/V8HTMLCanvasElementCustom.cpp:50: static bool
> determineAlpha(v8::Handle<v8::Value> alphaValue)
> We are going to need a more generic version of this function to handle all the
> other attributes that suffer from the same bug with WebGL.  Suggestion:
> template<T> static T readDictionaryValue(v8::Handle<v8::Value>, const T&
> default)
> 
>
https://codereview.chromium.org/290143007/diff/20001/Source/bindings/v8/custo...
> Source/bindings/v8/custom/V8HTMLCanvasElementCustom.cpp:74:
> webGLAttributes->setAlpha(determineAlpha(jsAttributes->Get(alpha)));
> Theroriginal bug was primarily about 2D canvas. If you are going to fix the
> webGL case as well, you should add a test for it, and you should also fix the
> other attributes that have the same problem.

Thank you for review :)

Now that I think about it again, helper function seems to be unnecessary.
If its value is undefined or null, we just have only to leave the value.

Could you please review again?

[Justin Novosad, 2014-05-21 14:09:19]

lgtm

[haraken, 2014-05-21 14:39:43]

LGTM for bindings/

[zino, 2014-05-21 15:09:58]

Thanks to everyone :)

[zino, 2014-05-21 15:10:08]

The CQ bit was checked by jinho.bang@samsung.com

[commit-bot: I haz the power, 2014-05-21 17:50:39]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jinho.bang@samsung.com/290143007/40001

[commit-bot: I haz the power, 2014-05-22 02:13:46]

Change committed as 174508