Bug 85836 - [chromium] move event_listener_unittest to webkit_unit_tests
Summary: [chromium] move event_listener_unittest to webkit_unit_tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tony Chang
URL:
Keywords:
Depends on: 85850
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-07 15:26 PDT by Tony Chang
Modified: 2012-05-08 10:05 PDT (History)
4 users (show)

See Also:


Attachments
Patch (14.00 KB, patch)
2012-05-07 15:45 PDT, Tony Chang
no flags Details | Formatted Diff | Diff
Patch for landing (14.02 KB, patch)
2012-05-07 15:52 PDT, Tony Chang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tony Chang 2012-05-07 15:26:57 PDT
[chromium] move event_listener_unittest to webkit_unit_tests
Comment 1 Tony Chang 2012-05-07 15:45:47 PDT
Created attachment 140609 [details]
Patch
Comment 2 Tony Chang 2012-05-07 15:46:27 PDT
First step in fixing https://code.google.com/p/chromium/issues/detail?id=126514 .
Comment 3 James Robinson 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
Comment 4 Tony Chang 2012-05-07 15:52:49 PDT
Created attachment 140610 [details]
Patch for landing
Comment 5 Tony Chang 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!
Comment 6 WebKit Review Bot 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>
Comment 7 WebKit Review Bot 2012-05-07 17:05:10 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Kent Tamura 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'
Comment 9 James Robinson 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
Comment 10 WebKit Review Bot 2012-05-07 18:46:01 PDT
Re-opened since this is blocked by 85850
Comment 11 Kent Tamura 2012-05-07 18:53:29 PDT
Rolled out by http://trac.webkit.org/changeset/116383
Comment 12 Tony Chang 2012-05-08 10:05:19 PDT
Committed r116433: <http://trac.webkit.org/changeset/116433>
Comment 13 Tony Chang 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.