Bug 110606 – [Chromium][DRT] DumpRenderTree TestShell::initialize should take Platform* now that WebKitPlatformSupport is empty

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Mark Pilgrim (Google)]

[Chromium] DumpRenderTree TestShell::initialize should take Platform* now that WebKitPlatformSupport is empty

[Mark Pilgrim (Google)]

Created attachment 189768 [details]
Patch

[Kentaro Hara]

Comment on attachment 189768 [details]
Patch

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

> Tools/DumpRenderTree/chromium/MockPlatform.cpp:32
> +#include "MockWebKitPlatformSupport.h"

MockPlatformSupport.h ?

> Tools/DumpRenderTree/chromium/MockPlatform.cpp:42
> +PassOwnPtr<MockWebKitPlatformSupport> MockWebKitPlatformSupport::create()

MockPlatformSupport? The same comment for other parts.

> Tools/DumpRenderTree/chromium/MockPlatform.h:31
> +#ifndef MockWebKitPlatformSupport_h

MockPlatform ?

> Tools/DumpRenderTree/chromium/MockPlatform.h:42
> +class MockWebKitPlatformSupport : public WebKit::Platform {

MockPlatform ?

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Adam Barth]

Comment on attachment 189768 [details]
Patch

Haraken, I'm not sure I understand your comments.  We're dropping both the "WebKit" and the "Support" from the names.  The old concept was called WebKit::WebKitPlatformSupport, but we've renamed it in the API to WebKit::Platform.

[Kentaro Hara]

Comment on attachment 189768 [details]
Patch

Sorry, it looks like I added strange comments to wrong places... I wanted to say it looks strange to have 'class MockWebKitPlatformSupport' in MockPlatform.h.

[Adam Barth]

> Sorry, it looks like I added strange comments to wrong places... I wanted to say it looks strange to have 'class MockWebKitPlatformSupport' in MockPlatform.h.

Notice that "Tools/DumpRenderTree/chromium/MockPlatform.h" and "Tools/DumpRenderTree/chromium/MockPlatform.cpp" appear twice in the diff.  The reason is because Mark has "svn mv" the files and then edited them.  The first appearance in the diff shows that they were moved and the second shows the edits.  It's confusing to read the diff, but he has actually made the changes you've asked for.  :)

[Kentaro Hara]

(In reply to comment #6)
> Notice that "Tools/DumpRenderTree/chromium/MockPlatform.h" and "Tools/DumpRenderTree/chromium/MockPlatform.cpp" appear twice in the diff.  The reason is because Mark has "svn mv" the files and then edited them.  The first appearance in the diff shows that they were moved and the second shows the edits.  It's confusing to read the diff, but he has actually made the changes you've asked for.  :)

Ah, understood. Sorry for all the confusion!

[WebKit Review Bot]

Comment on attachment 189768 [details]
Patch

Clearing flags on attachment: 189768

Committed r143852: <http://trac.webkit.org/changeset/143852>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.