Bug 88478

Summary: remove ChromiumDriver from NRWT
Product: WebKit Reporter: Dirk Pranke <dpranke>
Component: Tools / TestsAssignee: Xianzhu Wang <wangxianzhu>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, jnd, jochen, ojan, tony, wangxianzhu, webkit.review.bot, zhenghao
Priority: P2 Keywords: NRWT
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 88542    
Bug Blocks:    
Attachments:
Description Flags
Patch none

Description Dirk Pranke 2012-06-06 18:14:09 PDT
It seems like it's safe to remove the old test_shell code from the desktop chromium ports, as the webkit/drt output has run happily from some time.

However, it looks like the android port might still be using the old style interface? I'm not quite sure since I don't know what the state of the upstreaming is and if/how they specify the --test-shell command line anywhere to trigger the old behavior.

Can someone more familiar with the Android port confirm that we can either delete that code outright and just have ChromiumAndroidDriver subclass WebKitDriver, or merge all of the ChromiumDriver code into ChromiumAndroidDriver?
Comment 1 Adam Barth 2012-06-06 18:18:23 PDT
The chromium-android port does appear to be using --test-shell.  I'm not sure what's involved in making it work without that flag.
Comment 2 Dirk Pranke 2012-06-06 18:21:57 PDT
there's no real urgency on this change, but it might be nice to at least move the class over to reduce any potential confusion on the desktop side.
Comment 3 Dirk Pranke 2012-06-20 14:38:41 PDT
*** Bug 86928 has been marked as a duplicate of this bug. ***
Comment 4 Dirk Pranke 2012-07-13 19:30:23 PDT
resetting the owner in case someone else wants to take a look, as these bugs aren't on my immediate to-do list.
Comment 5 Xianzhu Wang 2012-07-16 14:11:39 PDT
I'll do this after finishing bug 88542.

Dirk, could you review the patch of bug 88542?
Comment 6 Dirk Pranke 2012-07-16 14:45:37 PDT
(In reply to comment #5)
> I'll do this after finishing bug 88542.
> 
> Dirk, could you review the patch of bug 88542?

Sure, posted comments there.
Comment 7 Xianzhu Wang 2012-07-16 18:06:14 PDT
Created attachment 152667 [details]
Patch
Comment 8 Dirk Pranke 2012-07-16 19:21:04 PDT
Comment on attachment 152667 [details]
Patch

yay!
Comment 9 WebKit Review Bot 2012-07-17 10:42:31 PDT
Comment on attachment 152667 [details]
Patch

Clearing flags on attachment: 152667

Committed r122855: <http://trac.webkit.org/changeset/122855>
Comment 10 WebKit Review Bot 2012-07-17 10:42:35 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Adam Barth 2012-07-17 11:04:22 PDT
@jochen: This is the patch I mentioned that removes support for test shell mode.
Comment 12 Adam Barth 2012-07-17 11:04:41 PDT
Is there code in DumpRenderTree to delete now too?
Comment 13 Xianzhu Wang 2012-07-17 11:05:59 PDT
(In reply to comment #12)
> Is there code in DumpRenderTree to delete now too?

That's in bug 86927.