Make getComputedStyle throw for incorrect first parameter

Make getComputedStyle throw for incorrect first parameter

When the first parameter is null, undefined or not an Element
type, throw an exception.

Matches Firefox and IE behavior.

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

[rwlbuis, 2014-11-21 23:17:57]

rob.buis@samsung.com changed reviewers:
+ tkent@chromium.org

[rwlbuis, 2014-11-21 23:17:57]

PTAL

[tkent, 2014-11-25 01:18:25]

> Matches Firefox behavior.

What about IE and the standard?

[rwlbuis, 2014-11-25 19:24:12]

On 2014/11/25 01:18:25, tkent wrote:
> > Matches Firefox behavior.
> 
> What about IE and the standard?

Sorry, should have checked. IE8 does not run the testcase in the bug, but in
IE11 it passes. I updated the commit message.

[rwlbuis, 2014-11-25 20:36:12]

On 2014/11/25 01:18:25, tkent wrote:
> > Matches Firefox behavior.
> 
> What about IE and the standard?

There does not seem to be anything about exception throwing  for
getComputedStyle in the
specs. I can file a bug for this if you like.

I now made a V2 version where the type checking is done in the bindings layer, I
think I
like that one better since we can remove the null check in
LocalDOMWindow::getComputedStyle.
Let me know what you think.

[tkent, 2014-11-25 22:59:28]

ok, let's go ahead.

https://codereview.chromium.org/752713002/diff/20001/LayoutTests/fast/css/get...
File
LayoutTests/fast/css/getComputedStyle/computed-style-non-element-parameter.html
(right):

https://codereview.chromium.org/752713002/diff/20001/LayoutTests/fast/css/get...
LayoutTests/fast/css/getComputedStyle/computed-style-non-element-parameter.html:3:
if (window.testRunner)
Indent the whole content of <script> is not helpful.

https://codereview.chromium.org/752713002/diff/20001/LayoutTests/fast/css/get...
LayoutTests/fast/css/getComputedStyle/computed-style-non-element-parameter.html:9:
noException = false;
Please import ../../../resources/js-test.js, and use
shouldNotThrow('getComputedStyle(null)').

https://codereview.chromium.org/752713002/diff/20001/Source/core/frame/Window...
File Source/core/frame/Window.idl (right):

https://codereview.chromium.org/752713002/diff/20001/Source/core/frame/Window...
Source/core/frame/Window.idl:141: [TypeChecking=Interface]CSSStyleDeclaration
getComputedStyle([Default=Undefined] optional Element element,
nit: add a space between ']' and 'C'

[rwlbuis, 2014-11-25 23:29:01]

On 2014/11/25 22:59:28, tkent wrote:
> ok, let's go ahead.
> 
>
https://codereview.chromium.org/752713002/diff/20001/LayoutTests/fast/css/get...
> File
>
LayoutTests/fast/css/getComputedStyle/computed-style-non-element-parameter.html
> (right):
> 
>
https://codereview.chromium.org/752713002/diff/20001/LayoutTests/fast/css/get...
>
LayoutTests/fast/css/getComputedStyle/computed-style-non-element-parameter.html:3:
> if (window.testRunner)
> Indent the whole content of <script> is not helpful.
> 
>
https://codereview.chromium.org/752713002/diff/20001/LayoutTests/fast/css/get...
>
LayoutTests/fast/css/getComputedStyle/computed-style-non-element-parameter.html:9:
> noException = false;
> Please import ../../../resources/js-test.js, and use
> shouldNotThrow('getComputedStyle(null)').
> 
>
https://codereview.chromium.org/752713002/diff/20001/Source/core/frame/Window...
> File Source/core/frame/Window.idl (right):
> 
>
https://codereview.chromium.org/752713002/diff/20001/Source/core/frame/Window...
> Source/core/frame/Window.idl:141: [TypeChecking=Interface]CSSStyleDeclaration
> getComputedStyle([Default=Undefined] optional Element element,
> nit: add a space between ']' and 'C'

Thanks! These issues should now be fixed with Patch Set 3.

[tkent, 2014-11-25 23:35:13]

The CQ bit was checked by tkent@chromium.org

[tkent, 2014-11-25 23:35:14]

lgtm

[commit-bot: I haz the power, 2014-11-25 23:35:31]

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

[commit-bot: I haz the power, 2014-11-26 00:57:20]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=185988