Implement UIEvent constructor

• Component: WebCore JavaScript
• Status: RESOLVED FIXED

[Kentaro Hara]

Editor's draft: https://dvcs.w3.org/hg/d4e/raw-file/tip/source_respec.htm

UIEvent constructor is implemented under a DOM4_EVENTS_CONSTRUCTOR flag, which is enabled on Chromium only.

[Kentaro Hara]

Created attachment 183728 [details]
Patch

[Sam Weinig]

I don't think an #ifdef is needed for such a simple change (I don't think we added one when we added Event constructors).

[Kentaro Hara]

(In reply to comment #2)
> I don't think an #ifdef is needed for such a simple change (I don't think we added one when we added Event constructors).

But in webkit-dev, maciej asked us to implement it under a flag because it's still on an editor's draft. (If you want, I can enable the flag in Safari too once I implemented all events.)

[Kentaro Hara]

Created attachment 183856 [details]
Patch

[Sam Weinig]

(In reply to comment #3)
> (In reply to comment #2)
> > I don't think an #ifdef is needed for such a simple change (I don't think we added one when we added Event constructors).
> 
> But in webkit-dev, maciej asked us to implement it under a flag because it's still on an editor's draft. (If you want, I can enable the flag in Safari too once I implemented all events.)

I talked with Maciej, and rescind my objection.

[Build Bot]

Comment on attachment 183856 [details]
Patch

Attachment 183856 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/16041280

New failing tests:
fast/events/constructors/ui-event-constructor.html

[Kentaro Hara]

Created attachment 183890 [details]
Patch

[Build Bot]

Comment on attachment 183890 [details]
Patch

Attachment 183890 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/16040389

New failing tests:
fast/events/constructors/ui-event-constructor.html

[Kentaro Hara]

hmm, DOM4_EVENTS_CONSTRUCTOR is not enabled on Safari... Looks like I'm missing something.

[Kentaro Hara]

Created attachment 183896 [details]
Patch

[Kentaro Hara]

Created attachment 183908 [details]
Patch

[Adam Barth]

Comment on attachment 183908 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183908&action=review

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:-409
> -        my $conditionalString = $codeGenerator->GenerateConstructorConditionalString($interface);
> -        push(@headerContent, "#if $conditionalString\n") if $conditionalString;

I'm surprised you're reverting these changes.

> Source/WebCore/dom/UIEvent.idl:21
> +    ConstructorConditional=DOM4_EVENTS_CONSTRUCTOR,

Especially since you're using the attribute here...

[Adam Barth]

I must be confused.  I'll look again in the morning.

[Kentaro Hara]

Comment on attachment 183908 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183908&action=review

>> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:-409
>> -        push(@headerContent, "#if $conditionalString\n") if $conditionalString;
> 
> I'm surprised you're reverting these changes.

This is just a nit.

Before (Compile fails if a flag is disabled):

  // V8XXXEvent.h
  #if ENABLE(...)
  static Handle<Value> constructorCallback();
  #endif

  // V8XXXEvent.cpp
  Handle<Value> constructorCallback() {
     ...
  }

  static void ConfigureV8XXXEventTemplate() {
    ...
  #if ENABLE(...)
    desc->SetCallHandler(V8XXXEvent::constructorCallback);
  #endif
    ...
  }

After:

  // V8XXXEvent.h
  static Handle<Value> constructorCallback();

  // V8XXXEvent.cpp
  Handle<Value> constructorCallback() {
     ...
  }

  static void ConfigureV8XXXEventTemplate() {
    ...
  #if ENABLE(...)
    desc->SetCallHandler(V8XXXEvent::constructorCallback);
  #endif
    ...
  }

I just didn't want to surround the definition of constructorCallback() because it slightly messes up CodeGeneratoV8.pm.

[Adam Barth]

Ok.  I'll look again when less tired.  :)

[Build Bot]

Comment on attachment 183908 [details]
Patch

Attachment 183908 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/16035632

New failing tests:
fast/dom/dom-constructors.html
fast/dom/constructed-objects-prototypes.html

[Adam Barth]

Comment on attachment 183908 [details]
Patch

This looks good to me, but there seems to be some trouble with the compile flag on apple-mac.

[Build Bot]

Comment on attachment 183908 [details]
Patch

Attachment 183908 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/16033711

New failing tests:
fast/dom/dom-constructors.html
fast/dom/constructed-objects-prototypes.html

[Kentaro Hara]

Created attachment 184065 [details]
patch for landing

[WebKit Review Bot]

Comment on attachment 184065 [details]
patch for landing

Rejecting attachment 184065 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
utTests/platform/efl/TestExpectations
Hunk #1 succeeded at 1232 (offset -5 lines).
patching file LayoutTests/platform/gtk/TestExpectations
Hunk #1 succeeded at 364 with fuzz 2 (offset -8 lines).
patching file LayoutTests/platform/qt/TestExpectations
patching file LayoutTests/platform/win/TestExpectations
patching file LayoutTests/platform/wincairo/TestExpectations

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/16063265

[Kentaro Hara]

Created attachment 184067 [details]
patch for landing

[WebKit Review Bot]

Comment on attachment 184067 [details]
patch for landing

Rejecting attachment 184067 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
um/third_party/v8-i18n --revision 159 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
54>At revision 159.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...
Total errors found: 0 in 5 files

Full output: http://queues.webkit.org/results/16040894

[WebKit Review Bot]

Comment on attachment 184067 [details]
patch for landing

Rejecting attachment 184067 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
WebKit/chromium/v8 --revision 13451 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
53>At revision 13451.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...
Total errors found: 0 in 5 files

Full output: http://queues.webkit.org/results/16038869

[Kentaro Hara]

Committed r140493: <http://trac.webkit.org/changeset/140493>