Rework dispatchEvent so it is consistent for isTrusted support.

Rework dispatchEvent so it is consistent for isTrusted support.

Make dispatchEvent a non-virtual method on EventTarget; make
the current dispatchEvent a protected method and rename it to
dispatchEventInternal.

The goal of this is that there are two main entry points to dispatching
events.
1) dispatchEventForBindings (from JS)
2) dispatchEvent() (from C++)

The trusted event code will take advantage of this to assign the
trusted value in these implementations; so this change will make more
sense in the context of that change.

This CL has no behavior changes.

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

[dtapuska, 2015-07-14 14:19:19]

dtapuska@chromium.org changed reviewers:
+ tkent@chromium.org

[dtapuska, 2015-07-14 21:39:09]

On 2015/07/14 14:19:19, Dave Tapuska wrote:

I'm investigating windows build bot failures.

[tkent, 2015-07-14 23:57:01]

lgtm.
If the fix for Windows bot failures is trivial, please go ahead landing.

[tkent, 2015-07-15 00:00:54]

https://codereview.chromium.org/1241613004/diff/1/Source/core/dom/Node.h
File Source/core/dom/Node.h (right):

https://codereview.chromium.org/1241613004/diff/1/Source/core/dom/Node.h#newc...
Source/core/dom/Node.h:628: void
dispatchEventDispatchMediator(PassRefPtrWillBeRawPtr<EventDispatchMediator>);
This looks unnecessary.

https://codereview.chromium.org/1241613004/diff/1/Source/modules/indexeddb/ID...
File Source/modules/indexeddb/IDBDatabase.h (right):

https://codereview.chromium.org/1241613004/diff/1/Source/modules/indexeddb/ID...
Source/modules/indexeddb/IDBDatabase.h:134: bool
dispatchEventInternal(PassRefPtrWillBeRawPtr<Event>) override;
I recommend to add a comment indicating what this overrides.  e.g.
    // EventTarget
Because the original code has such comment.

Same for other header files.

[dtapuska, 2015-07-15 19:33:41]

On 2015/07/15 00:00:54, tkent wrote:
> https://codereview.chromium.org/1241613004/diff/1/Source/core/dom/Node.h
> File Source/core/dom/Node.h (right):
> 
>
https://codereview.chromium.org/1241613004/diff/1/Source/core/dom/Node.h#newc...
> Source/core/dom/Node.h:628: void
> dispatchEventDispatchMediator(PassRefPtrWillBeRawPtr<EventDispatchMediator>);
> This looks unnecessary.
> 
>
https://codereview.chromium.org/1241613004/diff/1/Source/modules/indexeddb/ID...
> File Source/modules/indexeddb/IDBDatabase.h (right):
> 
>
https://codereview.chromium.org/1241613004/diff/1/Source/modules/indexeddb/ID...
> Source/modules/indexeddb/IDBDatabase.h:134: bool
> dispatchEventInternal(PassRefPtrWillBeRawPtr<Event>) override;
> I recommend to add a comment indicating what this overrides.  e.g.
>     // EventTarget
> Because the original code has such comment.
> 
> Same for other header files.

Adding the "virtual" keyword back on the EventTarget.h dispatchEvent method
makes it not crash on Windows.

I'm not certain why this causes an issue on windows; the object seems alive.
Could this be a compiler bug? Or a cross thread issue?

[dtapuska, 2015-07-15 22:34:42]

https://codereview.chromium.org/1241613004/diff/1/Source/core/dom/Node.h
File Source/core/dom/Node.h (right):

https://codereview.chromium.org/1241613004/diff/1/Source/core/dom/Node.h#newc...
Source/core/dom/Node.h:628: void
dispatchEventDispatchMediator(PassRefPtrWillBeRawPtr<EventDispatchMediator>);
On 2015/07/15 00:00:54, tkent wrote:
> This looks unnecessary.

Done.

https://codereview.chromium.org/1241613004/diff/1/Source/modules/indexeddb/ID...
File Source/modules/indexeddb/IDBDatabase.h (right):

https://codereview.chromium.org/1241613004/diff/1/Source/modules/indexeddb/ID...
Source/modules/indexeddb/IDBDatabase.h:134: bool
dispatchEventInternal(PassRefPtrWillBeRawPtr<Event>) override;
On 2015/07/15 00:00:54, tkent wrote:
> I recommend to add a comment indicating what this overrides.  e.g.
>     // EventTarget
> Because the original code has such comment.
> 
> Same for other header files.

Done.

[tkent, 2015-07-16 00:22:02]

The CQ bit was checked by tkent@chromium.org

[tkent, 2015-07-16 00:22:03]

lgtm

https://codereview.chromium.org/1241613004/diff/20001/Source/core/workers/Wor...
File Source/core/workers/WorkerEventQueue.cpp (right):

https://codereview.chromium.org/1241613004/diff/20001/Source/core/workers/Wor...
Source/core/workers/WorkerEventQueue.cpp:80: // before the target is queried.
Wow, nice detective work.

[commit-bot: I haz the power, 2015-07-16 00:22:45]

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

[commit-bot: I haz the power, 2015-07-16 00:31:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-07-16 00:31:11]

Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_chromium_gn_rel/bu...)

[tkent, 2015-07-16 00:37:30]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2015-07-16 00:37:49]

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

[commit-bot: I haz the power, 2015-07-16 00:45:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-07-16 00:45:30]

Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_chromium_gn_rel/bu...)

[tkent, 2015-07-16 01:05:26]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2015-07-16 01:05:39]

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

[commit-bot: I haz the power, 2015-07-16 01:13:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-07-16 01:13:30]

Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_chromium_gn_rel/bu...)

[dtapuska, 2015-07-16 01:36:06]

The CQ bit was checked by dtapuska@chromium.org

[commit-bot: I haz the power, 2015-07-16 01:36:12]

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

[commit-bot: I haz the power, 2015-07-16 01:55:06]

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

[grt (UTC plus 2), 2015-07-16 13:40:39]

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

[grt (UTC plus 2), 2015-07-16 13:40:40]

https://codereview.chromium.org/1241613004/diff/20001/Source/core/html/track/...
File Source/core/html/track/TextTrackCue.h (right):

https://codereview.chromium.org/1241613004/diff/20001/Source/core/html/track/...
Source/core/html/track/TextTrackCue.h:112: bool
dispatchEventInternal(PassRefPtrWillBeRawPtr<Event>) override;
"override" here is causing a compile error on chromium.fyi Chromium Builder:

c:\b\build\slave\chromium-rel-builder\build\src\third_party\webkit\source\core\html\track\texttrackcue.h(112):error
C3668: 'blink::TextTrackCue::dispatchEventInternal': method with override
specifier 'override' did not override any base class methods

please fix.

[dtapuska, 2015-07-16 13:51:27]

On 2015/07/16 13:40:40, grt wrote:
>
https://codereview.chromium.org/1241613004/diff/20001/Source/core/html/track/...
> File Source/core/html/track/TextTrackCue.h (right):
> 
>
https://codereview.chromium.org/1241613004/diff/20001/Source/core/html/track/...
> Source/core/html/track/TextTrackCue.h:112: bool
> dispatchEventInternal(PassRefPtrWillBeRawPtr<Event>) override;
> "override" here is causing a compile error on chromium.fyi Chromium Builder:
> 
>
c:\b\build\slave\chromium-rel-builder\build\src\third_party\webkit\source\core\html\track\texttrackcue.h(112):error
> C3668: 'blink::TextTrackCue::dispatchEventInternal': method with override
> specifier 'override' did not override any base class methods
> 
> please fix.

You sure something isn't wrong with this builder? (Caching of precompiled
headers?)

There is another failure as well...
c:\b\build\slave\chromium-rel-builder\build\src\third_party\webkit\source\core\fetch\scriptresource.h(60):
error C3668: 'blink::ScriptResource::destroyDecodedDataForFailedRevalidation':
method with override specifier 'override' did not override any base class
methods

The error makes no logical sense... TextTrackCue.h derives from
EventTargetWithInlineData which derives from EventTarget which has the virtual
method.

[grt (UTC plus 2), 2015-07-16 14:15:59]

On 2015/07/16 13:51:27, Dave Tapuska wrote:
> On 2015/07/16 13:40:40, grt wrote:
> >
>
https://codereview.chromium.org/1241613004/diff/20001/Source/core/html/track/...
> > File Source/core/html/track/TextTrackCue.h (right):
> > 
> >
>
https://codereview.chromium.org/1241613004/diff/20001/Source/core/html/track/...
> > Source/core/html/track/TextTrackCue.h:112: bool
> > dispatchEventInternal(PassRefPtrWillBeRawPtr<Event>) override;
> > "override" here is causing a compile error on chromium.fyi Chromium Builder:
> > 
> >
>
c:\b\build\slave\chromium-rel-builder\build\src\third_party\webkit\source\core\html\track\texttrackcue.h(112):error
> > C3668: 'blink::TextTrackCue::dispatchEventInternal': method with override
> > specifier 'override' did not override any base class methods
> > 
> > please fix.
> 
> You sure something isn't wrong with this builder? (Caching of precompiled
> headers?)
> 
> There is another failure as well...
>
c:\b\build\slave\chromium-rel-builder\build\src\third_party\webkit\source\core\fetch\scriptresource.h(60):
> error C3668: 'blink::ScriptResource::destroyDecodedDataForFailedRevalidation':
> method with override specifier 'override' did not override any base class
> methods
> 
> The error makes no logical sense... TextTrackCue.h derives from
> EventTargetWithInlineData which derives from EventTarget which has the virtual
> method.

Ah, I see that now. Freaky and sad. Thanks for taking a look.

[Daniel Bratell, 2015-07-16 14:46:39]

On 2015/07/16 14:15:59, grt wrote:
> On 2015/07/16 13:51:27, Dave Tapuska wrote:
> > On 2015/07/16 13:40:40, grt wrote:
> > >
> >
>
https://codereview.chromium.org/1241613004/diff/20001/Source/core/html/track/...
> > > File Source/core/html/track/TextTrackCue.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1241613004/diff/20001/Source/core/html/track/...
> > > Source/core/html/track/TextTrackCue.h:112: bool
> > > dispatchEventInternal(PassRefPtrWillBeRawPtr<Event>) override;
> > > "override" here is causing a compile error on chromium.fyi Chromium
Builder:
> > > 
> > >
> >
>
c:\b\build\slave\chromium-rel-builder\build\src\third_party\webkit\source\core\html\track\texttrackcue.h(112):error
> > > C3668: 'blink::TextTrackCue::dispatchEventInternal': method with override
> > > specifier 'override' did not override any base class methods
> > > 
> > > please fix.
> > 
> > You sure something isn't wrong with this builder? (Caching of precompiled
> > headers?)
> > 
> > There is another failure as well...
> >
>
c:\b\build\slave\chromium-rel-builder\build\src\third_party\webkit\source\core\fetch\scriptresource.h(60):
> > error C3668:
'blink::ScriptResource::destroyDecodedDataForFailedRevalidation':
> > method with override specifier 'override' did not override any base class
> > methods
> > 
> > The error makes no logical sense... TextTrackCue.h derives from
> > EventTargetWithInlineData which derives from EventTarget which has the
virtual
> > method.
> 
> Ah, I see that now. Freaky and sad. Thanks for taking a look.

Is that builder not using goma? Goma would have disabled precompiled headers
completely.

[Daniel Bratell, 2015-07-16 14:46:44]

On 2015/07/16 14:15:59, grt wrote:
> On 2015/07/16 13:51:27, Dave Tapuska wrote:
> > On 2015/07/16 13:40:40, grt wrote:
> > >
> >
>
https://codereview.chromium.org/1241613004/diff/20001/Source/core/html/track/...
> > > File Source/core/html/track/TextTrackCue.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1241613004/diff/20001/Source/core/html/track/...
> > > Source/core/html/track/TextTrackCue.h:112: bool
> > > dispatchEventInternal(PassRefPtrWillBeRawPtr<Event>) override;
> > > "override" here is causing a compile error on chromium.fyi Chromium
Builder:
> > > 
> > >
> >
>
c:\b\build\slave\chromium-rel-builder\build\src\third_party\webkit\source\core\html\track\texttrackcue.h(112):error
> > > C3668: 'blink::TextTrackCue::dispatchEventInternal': method with override
> > > specifier 'override' did not override any base class methods
> > > 
> > > please fix.
> > 
> > You sure something isn't wrong with this builder? (Caching of precompiled
> > headers?)
> > 
> > There is another failure as well...
> >
>
c:\b\build\slave\chromium-rel-builder\build\src\third_party\webkit\source\core\fetch\scriptresource.h(60):
> > error C3668:
'blink::ScriptResource::destroyDecodedDataForFailedRevalidation':
> > method with override specifier 'override' did not override any base class
> > methods
> > 
> > The error makes no logical sense... TextTrackCue.h derives from
> > EventTargetWithInlineData which derives from EventTarget which has the
virtual
> > method.
> 
> Ah, I see that now. Freaky and sad. Thanks for taking a look.

Is that builder not using goma? Goma would have disabled precompiled headers
completely.