RESOLVED FIXED 85836
[chromium] move event_listener_unittest to webkit_unit_tests 
https://bugs.webkit.org/show_bug.cgi?id=85836
Summary [chromium] move event_listener_unittest to webkit_unit_tests
Tony Chang
Reported 2012-05-07 15:26:57 PDT
[chromium] move event_listener_unittest to webkit_unit_tests
Attachments
Patch (14.00 KB, patch)
2012-05-07 15:45 PDT, Tony Chang 		no flags
DetailsFormatted DiffDiff
Patch for landing (14.02 KB, patch)
2012-05-07 15:52 PDT, Tony Chang 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Tony Chang
Comment 1 2012-05-07 15:45:47 PDT
Created attachment 140609 [details] Patch
Tony Chang
Comment 2 2012-05-07 15:46:27 PDT
First step in fixing https://code.google.com/p/chromium/issues/detail?id=126514 .
James Robinson
Comment 3 2012-05-07 15:48:49 PDT
Comment on attachment 140609 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=140609&action=review Death to test_shell_tests (and test_shell)! > Source/WebKit/chromium/tests/EventListenerTest.cpp:78 > + std::string baseURL("http://www.test.com/"); nit: shouldn't we use example.com ? test.com is a real domain, example.com is an RFC 2606 (?) placeholder > Source/WebKit/chromium/tests/data/listener/mutation_event_listener.html:1 > +<html> <!DOCTYPE html> unless we need quirks mode for this test
Tony Chang
Comment 4 2012-05-07 15:52:49 PDT
Created attachment 140610 [details] Patch for landing
Tony Chang
Comment 5 2012-05-07 15:53:10 PDT
(In reply to comment #3) > (From update of attachment 140609 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=140609&action=review > > Death to test_shell_tests (and test_shell)! > > > Source/WebKit/chromium/tests/EventListenerTest.cpp:78 > > + std::string baseURL("http://www.test.com/"); > > nit: shouldn't we use example.com ? test.com is a real domain, example.com is an RFC 2606 (?) placeholder Fixed. > > Source/WebKit/chromium/tests/data/listener/mutation_event_listener.html:1 > > +<html> > > <!DOCTYPE html> unless we need quirks mode for this test Added. Thanks for the fast review!
WebKit Review Bot
Comment 6 2012-05-07 17:05:05 PDT
Comment on attachment 140610 [details] Patch for landing Clearing flags on attachment: 140610 Committed r116375: <http://trac.webkit.org/changeset/116375>
WebKit Review Bot
Comment 7 2012-05-07 17:05:10 PDT
All reviewed patches have been landed. Closing bug.
Kent Tamura
Comment 8 2012-05-07 18:42:59 PDT
The patch caused build errors on Chromium-win and Chromium-mac. I'm rolling it out soon. 1: .\tests\EventListenerTest.cpp(76) : error C2146: syntax error : missing ';' before identifier 'SetUp' 2: .\tests\EventListenerTest.cpp(76) : error C2182: 'override' : illegal use of type 'void' 3: .\tests\EventListenerTest.cpp(76) : error C2433: '`anonymous-namespace'::WebDOMEventListenerTest::override' : 'virtual' not permitted on data declarations 4: .\tests\EventListenerTest.cpp(77) : error C4430: missing type specifier - int assumed. Note: C++ does not support default-int 5: .\tests\EventListenerTest.cpp(85) : error C2146: syntax error : missing ';' before identifier 'TearDown' 6: .\tests\EventListenerTest.cpp(85) : error C2182: 'override' : illegal use of type 'void' 7: .\tests\EventListenerTest.cpp(85) : error C2433: '`anonymous-namespace'::WebDOMEventListenerTest::override' : 'virtual' not permitted on data declarations 8: .\tests\EventListenerTest.cpp(85) : error C2086: 'int `anonymous-namespace'::WebDOMEventListenerTest::override' : redefinition 9: .\tests\EventListenerTest.cpp(86) : error C4430: missing type specifier - int assumed. Note: C++ does not support default-int 10: .\tests\EventListenerTest.cpp(117) : error C2555: '`anonymous-namespace'::WebDOMEventListenerTest::SetUp': overriding virtual function return type differs and is not covariant from 'testing::Test::SetUp' 11: .\tests\EventListenerTest.cpp(117) : error C2555: '`anonymous-namespace'::WebDOMEventListenerTest::TearDown': overriding virtual function return type differs and is not covariant from 'testing::Test::TearDown'
James Robinson
Comment 9 2012-05-07 18:45:34 PDT
oh - OVERRIDE has to go after the function name, not before it. It's an easy mechanical fix if you want to do that instead of reverting
WebKit Review Bot
Comment 10 2012-05-07 18:46:01 PDT
Re-opened since this is blocked by 85850
Kent Tamura
Comment 11 2012-05-07 18:53:29 PDT
Rolled out by http://trac.webkit.org/changeset/116383
Tony Chang
Comment 12 2012-05-08 10:05:19 PDT
Committed r116433: <http://trac.webkit.org/changeset/116433>
Tony Chang
Comment 13 2012-05-08 10:05:52 PDT
(In reply to comment #12) > Committed r116433: <http://trac.webkit.org/changeset/116433> Landed with the syntax fix. I verified that it compiled using clang.
Note You need to log in before you can comment on or make changes to this bug.