Bug 115853 - Merge [NoInterfaceObject] and [OmitConstructor] extended attributes
Summary: Merge [NoInterfaceObject] and [OmitConstructor] extended attributes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on: 115714 116116 116147 116343 116520 116530 116660 117183 117204 117225
Blocks:
  Show dependency treegraph
 
Reported: 2013-05-09 02:41 PDT by Chris Dumez
Modified: 2013-06-05 03:49 PDT (History)
24 users (show)

See Also:


Attachments
Patch (37.18 KB, patch)
2013-06-04 08:03 PDT, Chris Dumez
ggaren: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (575.21 KB, application/zip)
2013-06-04 09:54 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 (550.80 KB, application/zip)
2013-06-04 10:02 PDT, Build Bot
no flags Details
Patch for landing (37.18 KB, patch)
2013-06-04 13:33 PDT, Chris Dumez
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 (556.50 KB, application/zip)
2013-06-04 16:19 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion (500.37 KB, application/zip)
2013-06-04 17:59 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion (494.91 KB, application/zip)
2013-06-04 19:12 PDT, Build Bot
no flags Details
Patch for landing (37.18 KB, patch)
2013-06-05 01:04 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch for landing (37.31 KB, patch)
2013-06-05 01:25 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2013-05-09 02:41:38 PDT
[NoInterfaceObject] and [OmitConstructor] IDL attributes seem to overlap. It would probably make sense to merge them as the constructor should be omitted if [NoInterfaceObject] is present.

It is best to remove [OmitConstructor] as as [NoInterfaceObject] is standard.
Comment 1 Chris Dumez 2013-05-09 04:05:02 PDT
There are some challenges with this currently because some interfaces have [NoInterfaceObject] extended attribute and a Constructor attribute is manually added to the Window interface for those.

This is currently needed for Constructors attributes that have a [CustomGetter] and for Constructors whose name does not match the interface name.

This is the case for WebSocket for example.
Comment 2 Ryosuke Niwa 2013-05-09 09:43:53 PDT
We should look at the Web IDL specification and match whatever they have. If I remember correctly, I think Blink has removed NoInterfaceObject.
Comment 3 Chris Dumez 2013-05-09 09:49:37 PDT
(In reply to comment #2)
> We should look at the Web IDL specification and match whatever they have. If I remember correctly, I think Blink has removed NoInterfaceObject.

As I explained, [NoInterfaceObject] is part of the Web IDL spec and I have just added support for it in both WebKit and Blink. However, [OmitConstructor] is not standard and was removed from Blink already.
Comment 4 Chris Dumez 2013-06-04 08:03:37 PDT
Created attachment 203698 [details]
Patch
Comment 5 Geoffrey Garen 2013-06-04 09:41:04 PDT
Comment on attachment 203698 [details]
Patch

r=me
Comment 6 Build Bot 2013-06-04 09:54:25 PDT
Comment on attachment 203698 [details]
Patch

Attachment 203698 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/701366

New failing tests:
fast/dom/wrapper-classes.html
Comment 7 Build Bot 2013-06-04 09:54:30 PDT
Created attachment 203706 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.3
Comment 8 Build Bot 2013-06-04 10:02:09 PDT
Comment on attachment 203698 [details]
Patch

Attachment 203698 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/704629

New failing tests:
fast/dom/wrapper-classes.html
Comment 9 Build Bot 2013-06-04 10:02:14 PDT
Created attachment 203707 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3
Comment 10 Chris Dumez 2013-06-04 10:31:54 PDT
(In reply to comment #8)
> (From update of attachment 203698 [details])
> Attachment 203698 [details] did not pass mac-wk2-ews (mac-wk2):
> Output: http://webkit-queues.appspot.com/results/704629
> 
> New failing tests:
> fast/dom/wrapper-classes.html

Addressing this failure in Bug 117204 (marked as dependency of this bug).
Comment 11 Chris Dumez 2013-06-04 13:33:36 PDT
Created attachment 203722 [details]
Patch for landing
Comment 12 Chris Dumez 2013-06-04 13:34:29 PDT
(In reply to comment #11)
> Created an attachment (id=203722) [details]
> Patch for landing

Reupload without change to make sure EWS is OK now that the dependency has landed.
Comment 13 Build Bot 2013-06-04 16:19:26 PDT
Comment on attachment 203722 [details]
Patch for landing

Attachment 203722 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/746443

New failing tests:
fast/dom/wrapper-classes.html
Comment 14 Build Bot 2013-06-04 16:19:29 PDT
Created attachment 203738 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3
Comment 15 Build Bot 2013-06-04 17:58:57 PDT
Comment on attachment 203722 [details]
Patch for landing

Attachment 203722 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/739850

New failing tests:
fast/dom/wrapper-classes.html
Comment 16 Build Bot 2013-06-04 17:59:02 PDT
Created attachment 203744 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.3
Comment 17 Build Bot 2013-06-04 19:11:55 PDT
Comment on attachment 203722 [details]
Patch for landing

Attachment 203722 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/700875

New failing tests:
fast/dom/wrapper-classes.html
Comment 18 Build Bot 2013-06-04 19:12:01 PDT
Created attachment 203748 [details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-03  Port: mac-mountainlion  Platform: Mac OS X 10.8.3
Comment 19 Chris Dumez 2013-06-04 23:03:50 PDT
(In reply to comment #17)
> (From update of attachment 203722 [details])
> Attachment 203722 [details] did not pass mac-ews (mac):
> Output: http://webkit-queues.appspot.com/results/700875
> 
> New failing tests:
> fast/dom/wrapper-classes.html

Addressing remaining failure in Bug 117225.
Comment 20 Chris Dumez 2013-06-05 01:04:31 PDT
Created attachment 203766 [details]
Patch for landing

Reupload without change for ews now that the dependency has landed.
Comment 21 Chris Dumez 2013-06-05 01:25:15 PDT
Created attachment 203768 [details]
Patch for landing

Rebased on master due to MemoryInfo.idl having moved.
Comment 22 WebKit Commit Bot 2013-06-05 03:46:32 PDT
Comment on attachment 203768 [details]
Patch for landing

Clearing flags on attachment: 203768

Committed r151207: <http://trac.webkit.org/changeset/151207>
Comment 23 WebKit Commit Bot 2013-06-05 03:46:38 PDT
All reviewed patches have been landed.  Closing bug.
Comment 24 Chris Dumez 2013-06-05 03:49:04 PDT
I Updated the WebKit IDL wiki accordingly:
https://trac.webkit.org/wiki/WebKitIDL?action=diff&version=121&old_version=120