Bug 164025 - [Web IDL] Add support for having string enumerations in their own IDL file
Summary: [Web IDL] Add support for having string enumerations in their own IDL file
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks: 164063
  Show dependency treegraph
 
Reported: 2016-10-26 11:33 PDT by Chris Dumez
Modified: 2016-10-27 09:20 PDT (History)
8 users (show)

See Also:


Attachments
WIP Patch (66.71 KB, patch)
2016-10-26 14:25 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP Patch (64.68 KB, patch)
2016-10-26 14:29 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP Patch (76.82 KB, patch)
2016-10-26 14:45 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-yosemite (975.15 KB, application/zip)
2016-10-26 15:50 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-yosemite-wk2 (1.13 MB, application/zip)
2016-10-26 15:53 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews113 for mac-yosemite (1.77 MB, application/zip)
2016-10-26 15:58 PDT, Build Bot
no flags Details
Patch (94.39 KB, patch)
2016-10-26 17:04 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (93.91 KB, patch)
2016-10-26 19:24 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (94.43 KB, patch)
2016-10-26 19:59 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (94.51 KB, patch)
2016-10-26 20:19 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (94.70 KB, patch)
2016-10-26 20:55 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (95.90 KB, patch)
2016-10-26 21:47 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 2016-10-26 11:33:50 PDT
Add support for having string enumerations in their own IDL file so that they can be shared.
Comment 1 Chris Dumez 2016-10-26 13:57:19 PDT
Will use this for IDBTransactionMode string enumeration.
Comment 2 Chris Dumez 2016-10-26 14:25:17 PDT
Created attachment 292959 [details]
WIP Patch
Comment 3 Chris Dumez 2016-10-26 14:29:23 PDT
Created attachment 292960 [details]
WIP Patch
Comment 4 Chris Dumez 2016-10-26 14:45:07 PDT
Created attachment 292962 [details]
WIP Patch
Comment 5 Build Bot 2016-10-26 15:50:10 PDT
Comment on attachment 292962 [details]
WIP Patch

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

New failing tests:
storage/indexeddb/modern/idbdatabase-transaction-failures.html
storage/indexeddb/exceptions.html
storage/indexeddb/exceptions-private.html
storage/indexeddb/transaction-basics-private.html
storage/indexeddb/transaction-basics.html
storage/indexeddb/modern/idbdatabase-transaction-failures-private.html
Comment 6 Build Bot 2016-10-26 15:50:13 PDT
Created attachment 292965 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 7 Build Bot 2016-10-26 15:53:56 PDT
Comment on attachment 292962 [details]
WIP Patch

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

New failing tests:
storage/indexeddb/modern/idbdatabase-transaction-failures.html
storage/indexeddb/exceptions.html
storage/indexeddb/exceptions-private.html
storage/indexeddb/transaction-basics-private.html
storage/indexeddb/transaction-basics.html
storage/indexeddb/modern/idbdatabase-transaction-failures-private.html
Comment 8 Build Bot 2016-10-26 15:53:59 PDT
Created attachment 292967 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 9 Build Bot 2016-10-26 15:58:23 PDT
Comment on attachment 292962 [details]
WIP Patch

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

New failing tests:
storage/indexeddb/modern/idbdatabase-transaction-failures.html
storage/indexeddb/exceptions.html
storage/indexeddb/exceptions-private.html
storage/indexeddb/transaction-basics-private.html
storage/indexeddb/transaction-basics.html
storage/indexeddb/modern/idbdatabase-transaction-failures-private.html
Comment 10 Build Bot 2016-10-26 15:58:26 PDT
Created attachment 292968 [details]
Archive of layout-test-results from ews113 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 11 Chris Dumez 2016-10-26 16:53:17 PDT
Windows is failing to build :/

c:\cygwin\home\buildbot\webkit\webkitbuild\release\derivedsources\webcore\JSIDBTransactionMode.h(30): error C2908: explicit specialization; 'WTF::Optional<WebCore::IDBTransactionMode> WebCore::parseEnumeration<WebCore::IDBTransactionMode>(JSC::ExecState &,JSC::JSValue)' has already been instantiated (compiling source file C:\cygwin\home\buildbot\WebKit\Source\WebCore\DerivedSources.cpp) [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebCore\WebCoreDerivedSources.vcxproj]
c:\cygwin\home\buildbot\webkit\webkitbuild\release\derivedsources\webcore\JSIDBTransactionMode.h(32): error C2908: explicit specialization; 'const char *WebCore::expectedEnumerationValues<WebCore::IDBTransactionMode>(void)' has already been instantiated (compiling source file C:\cygwin\home\buildbot\WebKit\Source\WebCore\DerivedSources.cpp) [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebCore\WebCoreDerivedSources.vcxproj]
Comment 12 Chris Dumez 2016-10-26 17:04:24 PDT
Created attachment 292975 [details]
Patch
Comment 13 Chris Dumez 2016-10-26 17:05:06 PDT
Should fix the tests, however, I have not figured out how to fix the Window "All In One" build yet.
Comment 14 Darin Adler 2016-10-26 17:45:31 PDT
Looks fantastic.
Comment 15 Chris Dumez 2016-10-26 19:24:26 PDT
Created attachment 292985 [details]
Patch
Comment 16 Chris Dumez 2016-10-26 19:59:53 PDT
Created attachment 292986 [details]
Patch
Comment 17 Chris Dumez 2016-10-26 20:19:02 PDT
Created attachment 292987 [details]
Patch
Comment 18 Chris Dumez 2016-10-26 20:55:53 PDT
Created attachment 292988 [details]
Patch
Comment 19 Chris Dumez 2016-10-26 21:31:58 PDT
I protected the header with "#if !ENABLE(ALLINONE_BUILD)" and now the errors are for the cpp file:

C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\DerivedSources\WebCore\JSIDBTransactionMode.cpp(58): error C2908: explicit specialization; 'WTF::Optional<WebCore::IDBTransactionMode> WebCore::parseEnumeration<WebCore::IDBTransactionMode>(JSC::ExecState &,JSC::JSValue)' has already been instantiated (compiling source file C:\cygwin\home\buildbot\WebKit\Source\WebCore\DerivedSources.cpp) [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebCore\WebCoreDerivedSources.vcxproj]
C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\DerivedSources\WebCore\JSIDBTransactionMode.cpp(75): error C2908: explicit specialization; 'const char *WebCore::expectedEnumerationValues<WebCore::IDBTransactionMode>(void)' has already been instantiated (compiling source file C:\cygwin\home\buildbot\WebKit\Source\WebCore\DerivedSources.cpp) [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebCore\WebCoreDerivedSources.vcxproj]

I don't quite understand how the template function can already be instantiated :/
Comment 20 Chris Dumez 2016-10-26 21:47:59 PDT
Created attachment 292992 [details]
Patch
Comment 21 Chris Dumez 2016-10-26 21:49:46 PDT
I think the issue is that the template function is instantiated / used before its explicit specialization is provided. This likely means I am not properly including "JSIDBTransactionMode.h" in some places. Trying to see if this fixes it.
Comment 22 Chris Dumez 2016-10-26 22:28:25 PDT
Comment on attachment 292992 [details]
Patch

OMG everything is green, even Windows!
Comment 23 Darin Adler 2016-10-26 22:39:24 PDT
Comment on attachment 292992 [details]
Patch

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

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:905
> +    # FIXME: A little ugly to have this be a side effect instead of a return value.

Iā€™m not sure why we have this FIXME in some places but not others, even though I was probably the first offender, the first person to write that comment. For example, teh function above has the identical issue with $headerIncludes{} but no comment.
Comment 24 WebKit Commit Bot 2016-10-26 23:03:03 PDT
Comment on attachment 292992 [details]
Patch

Clearing flags on attachment: 292992

Committed r207937: <http://trac.webkit.org/changeset/207937>
Comment 25 WebKit Commit Bot 2016-10-26 23:03:10 PDT
All reviewed patches have been landed.  Closing bug.