Support non-function object values assigned to EventHandler attributes

Support non-function object values assigned to EventHandler attributes

In HTML, the type EventHandler, used for event handler IDL attributes,
is defined

  [TreatNonObjectAsNull]
  callback EventHandlerNonNull = any (Event event);

  typedef EventHandlerNonNull? EventHandler;

Aside from treating non-object values as null, the extended attribute
means non-callable object values should be allowed by the attribute's
setter (and returned by its getter) and should be treated as if they
were no-op functions returning undefined.

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

[Jens Widell, 2015-04-09 14:14:08]

jl@opera.com changed reviewers:
+ haraken@chromium.org

[Jens Widell, 2015-04-09 14:14:08]

PTAL

[vivekg, 2015-04-09 14:39:13]

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

[vivekg, 2015-04-09 14:39:13]

I see the behavior in FF has been already working as per the test of this patch.
When I verified with opera (12.7 presto) its behavior matches current chrome
one. So its nice to see that every other browser is catching up with the specs.

https://codereview.chromium.org/1076783002/diff/20001/Source/bindings/core/v8...
File Source/bindings/core/v8/V8EventListener.cpp (right):

https://codereview.chromium.org/1076783002/diff/20001/Source/bindings/core/v8...
Source/bindings/core/v8/V8EventListener.cpp:52: if (listener.IsEmpty())
Is it a good idea to combine listener.IsEmpty() with isAttribute() check as both
of the return values are same?
Probably it doesn't save much either ways so just a quick qn.

[Jens Widell, 2015-04-09 14:49:04]

https://codereview.chromium.org/1076783002/diff/20001/Source/bindings/core/v8...
File Source/bindings/core/v8/V8EventListener.cpp (right):

https://codereview.chromium.org/1076783002/diff/20001/Source/bindings/core/v8...
Source/bindings/core/v8/V8EventListener.cpp:52: if (listener.IsEmpty())
On 2015/04/09 14:39:13, vivekg_ wrote:
> Is it a good idea to combine listener.IsEmpty() with isAttribute() check as
both
> of the return values are same?
> Probably it doesn't save much either ways so just a quick qn.

I don't see how we could easily do without having two 'listener->IsFunction()'
checks instead, and/or making the code more intricate (albeit functionally
equivalent.)

[vivekg, 2015-04-09 14:58:51]

On 2015/04/09 at 14:49:04, jl wrote:
>
https://codereview.chromium.org/1076783002/diff/20001/Source/bindings/core/v8...
> File Source/bindings/core/v8/V8EventListener.cpp (right):
> 
>
https://codereview.chromium.org/1076783002/diff/20001/Source/bindings/core/v8...
> Source/bindings/core/v8/V8EventListener.cpp:52: if (listener.IsEmpty())
> On 2015/04/09 14:39:13, vivekg_ wrote:
> > Is it a good idea to combine listener.IsEmpty() with isAttribute() check as
both
> > of the return values are same?
> > Probably it doesn't save much either ways so just a quick qn.
> 
> I don't see how we could easily do without having two 'listener->IsFunction()'
checks instead, and/or making the code more intricate (albeit functionally
equivalent.)

Yeah forget about my comment. I hit the send button and realized about it. :)

[haraken, 2015-04-09 23:35:14]

LGTM

[Jens Widell, 2015-04-10 04:40:21]

The CQ bit was checked by jl@opera.com

[commit-bot: I haz the power, 2015-04-10 04:40:33]

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

[commit-bot: I haz the power, 2015-04-10 06:24:11]

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