[Webkit-unassigned] [Bug 67590] Implement DRT support for Android.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=67590





--- Comment #14 from Hao Zheng <zhenghao at chromium.org>  2011-09-06 18:38:23 PST ---
(In reply to comment #11)
> (From update of attachment 106403 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=106403&action=review
> 
> > Source/WebKit/chromium/WebKit.gyp:1131
> > +                ['OS=="android"', {
> > +                    # FIXME(zhenghao): Reserve for upstream in the near future.
> > +                },{ # OS!="android"
> 
> This is weird.  Just flip your condition (OS!="android") and remove the else clause.

I will upstream some stuff in the first brace very soon. Is it acceptable to leave it there? If not, I can remove it.
My intention here is to facilitate our regular merge effort. It will generate minimum diff when we merge upstream code now.

> 
> > Tools/ChangeLog:11
> > +        - Wait until receiving '#READY' from DRT, so that DRT won't miss any
> > +        input commands before it starts.
> 
> Can you explain your architecture to me?  It sounds like DRT runs on Android, but it's not clear to me if new-run-webkit-tests runs on Android or the desktop.

Sure! I intend to elaborate on that when I upstream NRWT. Anyway, let me explain it here.

NRWT on host machine (linux/mac)
                 |
Android shell connecting host and device (adb shell)
                 |
DRT on Android

As Android shell is rather limited, we cannot run NRWT on it. 'adb shell' can forward stdin/stdout between host and device, but unfortunately it doesn't ensure integrity, like an unreliable 'pipe'.

> 
> It seems unfortunate that DRT needs to acknowledge that it has started so I was trying to brainstorm ways to avoid this.

If we send the first command to 'adb shell' and unfortunately DRT is not ready to receive stdin at that time, 'adb shell' will throw the command away. Then NRWT will wait for the DRT output, and DRT will wait for the NRWT input, which becomes a dead lock. That's why we need a way to make sure DRT is ready when NRWT sends the first command.

And 'adb shell' is also not reliable to send the EOF to DRT, so we add the 'QUIT' command.

> 
> > Tools/DumpRenderTree/chromium/DumpRenderTree.cpp:151
> > +    bool sendReady = false; // Effective only in testShellMode.
> 
> Maybe print a warning to stderr if --send-ready is used without --test-shell?

Ok.

> 
> > Tools/DumpRenderTree/chromium/TestShellAndroid.cpp:41
> > +static void AlarmHandler(int signatl)
> 
> Nit: Remove signatl (WebKit style normally just omits unused params)?

Ok.

> 
> > Tools/DumpRenderTree/chromium/TestShellAndroid.cpp:81
> > +{
> > +}
> 
> notImplemented()

As Steve said, I intentionally leave them empty. If I put notImplemented() there, DRT will output annoying warning every time the functions are reached.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.