Removes DontCheckEnums from some IDLs and fixes the corresponding enums.
Created attachment 61550 [details] Patch
Hi Darin, This is the first follow up to https://bugs.webkit.org/show_bug.cgi?id=42250, it fixes a few IDL / enums and allow generating compile-time checks for them. Would you mind taking a look please? Thanks, Marcus
Comment on attachment 61550 [details] Patch > enum orientType { > - VERTICAL = 0, > - HORIZONTAL = 1, > + HORIZONTAL = 0, > + VERTICAL = 1, > BOTH = 2 > }; This change seems risky. Was there any C++ code using these constants and thus getting things wrong? Can we construct a test that demonstrates we had them wrong? The rest of the changes seem obviously good.
Created attachment 61552 [details] Patch
(In reply to comment #3) > (From update of attachment 61550 [details]) > > enum orientType { > > - VERTICAL = 0, > > - HORIZONTAL = 1, > > + HORIZONTAL = 0, > > + VERTICAL = 1, > > BOTH = 2 > > }; > > This change seems risky. Was there any C++ code using these constants and thus getting things wrong? Can we construct a test that demonstrates we had them wrong? yes! added the test, but I'm not sure if it's the best place. let me know if you'd like it to be on a test of its own. > > The rest of the changes seem obviously good. cool, thanks!
Comment on attachment 61552 [details] Patch Rejecting patch 61552 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--ignore-tests', 'compositing', '--quiet']" exit_code: 1 Running build-dumprendertree Compiling Java tests make: Nothing to be done for `default'. Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Testing 20688 test cases. fast/events/overflow-events.html -> failed Exiting early after 1 failures. 7260 tests run. 133.90s total testing time 7259 test cases (99%) succeeded 1 test case (<1%) had incorrect layout Full output: http://webkit-commit-queue.appspot.com/results/3577024
Created attachment 61945 [details] Patch
(In reply to comment #3) > (From update of attachment 61550 [details]) > > enum orientType { > > - VERTICAL = 0, > > - HORIZONTAL = 1, > > + HORIZONTAL = 0, > > + VERTICAL = 1, > > BOTH = 2 > > }; > > This change seems risky. Was there any C++ code using these constants and thus getting things wrong? Can we construct a test that demonstrates we had them wrong? > > The rest of the changes seem obviously good. sorry to bother you again: there was an existing test (fast/events/overflow-events.html) that failed with this change. the issue being that it tested against the numeric hardcoded values, and looks like it assumed the C++ side rather than the IDL definitions. I changed the test to remove the hardcoded values and use the IDL-defined consts instead, would you mind taking another look? thanks!
Comment on attachment 61945 [details] Patch Seems to me that having a test that actually tests the integer values was valuable. Too bad it was expecting the wrong values! Changing the test to use the named constants makes the test more readable, but changes what it tests.
Comment on attachment 61945 [details] Patch Rejecting patch 61945 from commit-queue. Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Darin Adler', u'--force']" exit_code: 1 Last 500 characters of output: outTests/fast/events/script-tests/init-events.js patching file WebCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file WebCore/dom/Node.h patching file WebCore/dom/Node.idl patching file WebCore/dom/OverflowEvent.h patching file WebCore/dom/OverflowEvent.idl Hunk #1 FAILED at 23. 1 out of 1 hunk FAILED -- saving rejects to file WebCore/dom/OverflowEvent.idl.rej patching file WebCore/html/FileReader.cpp patching file WebCore/html/FileReader.h patching file WebCore/html/FileReader.idl Full output: http://queues.webkit.org/results/3592218
Created attachment 62817 [details] Patch
sorry about the delay. the latest patch merges with ToT and fixes the conflict issues reported above. another look please? (In reply to comment #11) > Created an attachment (id=62817) [details] > Patch
Comment on attachment 62817 [details] Patch Rejecting patch 62817 from commit-queue. Failed to run "[u'git', u'svn', u'dcommit']" exit_code: 1 Last 500 characters of output: .cpp M WebCore/html/FileReader.h M WebCore/html/FileReader.idl A repository hook failed: MERGE request failed on '/repository/webkit/trunk': Commit blocked by pre-commit hook (exit code 1) with output: The following files contain tab characters: trunk/LayoutTests/fast/events/overflow-events.html Please use spaces instead to indent. If you must commit a file with tabs, use svn propset to set the "allow-tabs" property. at /usr/local/git/libexec/git-core/git-svn line 570 Full output: http://queues.webkit.org/results/3632121
Comment on attachment 62817 [details] Patch Sorry, you seem to have hit an error in the commit queue. Let's try again.
Comment on attachment 62817 [details] Patch Oh, actually, you have tabs.
Created attachment 64073 [details] Patch for landing
Oh, thanks Adam!
thanks Adam! this went off my radar.. :( (In reply to comment #17) > Oh, thanks Adam!
Comment on attachment 64073 [details] Patch for landing Rejecting patch 64073 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--ignore-tests', 'compositing', '--quiet']" exit_code: 1 Last 500 characters of output: youtTests Testing 20828 test cases. media/video-size-intrinsic-scale.html -> timed out Sampling process 79668 for 10 seconds with 10 milliseconds of run time between samples Sampling completed, processing symbols... Sample analysis of process 79668 written to file /Users/eseidel/Library/Logs/DumpRenderTree/HangReport.txt Exiting early after 1 failures. 17216 tests run. 862.47s total testing time 17215 test cases (99%) succeeded 1 test case (<1%) timed out 36 test cases (<1%) had stderr output Full output: http://queues.webkit.org/results/3765063
Marcus, I guess just land manually?
(In reply to comment #20) > Marcus, I guess just land manually? landing manually now.
Committed r65226: <http://trac.webkit.org/changeset/65226>
The media timeouts are caused by some CoreVideo bug, which I thought was fixed. (Or at least they were last time they happened.) I haven't seen any timeouts since this bug, so maybe it was a WebCore change which was causing troubles.