android: Add browser type for webview shell to telemetry

android: Add browser type for webview shell to telemetry

This adds a browser type 'android-webview-shell' for the test
shell activity (AndroidWebView.apk), which is different from the existing
'android-webview' target that uses the device's system WebView for
testing.

Committed: https://crrev.com/8d9f0ee4fd09b912c8aca025ccc3c8e2c653f275
Cr-Commit-Position: refs/heads/master@{#292734}

[no sievers, 2014-08-28 21:01:25]

sievers@chromium.org changed reviewers:
+ epenner@chromium.org

[no sievers, 2014-08-28 21:01:31]

ptal

[tonyg, 2014-08-28 21:05:35]

tonyg@chromium.org changed reviewers:
+ tonyg@chromium.org

[tonyg, 2014-08-28 21:05:35]

lgtm

https://codereview.chromium.org/517573004/diff/1/tools/telemetry/telemetry/co...
File tools/telemetry/telemetry/core/backends/chrome/android_browser_backend.py
(right):

https://codereview.chromium.org/517573004/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/backends/chrome/android_browser_backend.py:169: 
uber-nit: 2 blank lines between top level definitions.

[epennerAtGoogle, 2014-08-28 21:49:20]

epenner@google.com changed reviewers:
+ benm@chromium.org, primiano@chromium.org

[epennerAtGoogle, 2014-08-28 21:50:58]

LGTM, however, if benm/primano can comment on the Android version maybe we
should add yet another one here. In the Android tree the WebViewShell has a
different package (com.android.webview.chromium.shell), so I've recently
discovered Chrome doesn't detect it... Should we add another one or change the
package in Chrome (or maybe remove the source from Chrome entirely?).

[no sievers, 2014-08-28 21:53:54]

https://codereview.chromium.org/517573004/diff/1/tools/telemetry/telemetry/co...
File tools/telemetry/telemetry/core/backends/chrome/android_browser_backend.py
(right):

https://codereview.chromium.org/517573004/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/backends/chrome/android_browser_backend.py:143:
cmdline_file='/data/local/tmp/webview-command-line',
Btw, I doubt this command line file is respected by the system WebView.

https://codereview.chromium.org/517573004/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/backends/chrome/android_browser_backend.py:169: 
On 2014/08/28 21:05:34, tonyg wrote:
> uber-nit: 2 blank lines between top level definitions.

Done.

[no sievers, 2014-08-28 21:54:42]

On 2014/08/28 21:50:58, epennerAtGoogle wrote:
> LGTM, however, if benm/primano can comment on the Android version maybe we
> should add yet another one here. In the Android tree the WebViewShell has a
> different package (com.android.webview.chromium.shell), so I've recently
> discovered Chrome doesn't detect it... Should we add another one or change the
> package in Chrome (or maybe remove the source from Chrome entirely?).

Maybe we can just make the name consistent again. Also using com.android.* for
the system WebView wrapper would make it more obvious which is which.

[benm (inactive), 2014-08-29 12:19:49]

benm@chromium.org changed reviewers:
+ hjd@chromium.org, mkosiba@chromium.org

[benm (inactive), 2014-08-29 12:19:49]

Hector and Marcin have been looking at removing the downstream system WebView
wrapper and moving it upstream to allow us to run tests; I think that would
replace the other webview shell, not the one that you are adding though. (As I
understand it this change is adding the ability to test an AwContents embedder,
not strictly webview)

[hjd, 2014-08-29 13:41:45]

So we (Marcin and I) recently moved the WebViewShell into Chrome and
renamed the package and browser name but I haven't yet removed the
Android version (I have a cl out) which has probably contributed to the
confusion. :( I think there are a lot of reasons to prefer to have it in
the Chrome tree which I won't get into unless people feel strongly about
removing it.

I support changing the name, we weren't super happy with it at the time,
maybe we could have system-android-webview, SystemWebViewShell,
system_webview_shell_apk (for the browser_type, APK name, gyp target)? given
that we're are hopefully soon going to have a SystemWebView (gyp:
system_webview_apk) 
apk which will be able to replace the the system WebView.

I'm more ambivalent about changing the package name root, I feel like stuff
that lives only in the Chrome tree should be "org.chromium.*".

Changing the browser_type/package of the current shell will break the
aosp_android_webview_perf and the webview_telemetry bot but just let me
know and I can fix them, we don't mind them being red for a bit.

Adding telemetry to the WebView shell sounds awesome! :)


(What I think the current situation is just to be clear: 
Target                                   browser_type           Tree     Tests  
    Package
gyp:android_webview_apk                  android-webview-shell  Chrome  
AwContents  org.chromium.android_webview.shell (this cl)
gyp:android_webview_telemetry_shell_apk  android-webview        Chrome   WebView
    org.chromium.telemetry_shell
make:WebViewShell                        [removed]              Android  WebView
    com.android.webview.chromium.shell (waiting on me to remove))

[hjd, 2014-08-29 13:46:03]

> (What I think the current situation is just to be clear: 
> Target                                   browser_type           Tree     Tests
 
>     Package
> gyp:android_webview_apk                  android-webview-shell  Chrome  
> AwContents  org.chromium.android_webview.shell (this cl)
> gyp:android_webview_telemetry_shell_apk  android-webview        Chrome  
WebView
>     org.chromium.telemetry_shell
> make:WebViewShell                        [removed]              Android 
WebView
>     com.android.webview.chromium.shell (waiting on me to remove))

1)
Target:       gyp:android_webview_apk
browser_type: android-webview-shell
Tree:         Chrome
Tests:        AwContents
Package:      org.chromium.android_webview.shell
(this cl)

2)
Target:       gyp:android_webview_telemetry_shell_apk
browser_type: android-webview
Tree:         Chrome
Tests:        WebView
Package:      org.chromium.telemetry_shell

3)
Target:       make:WebViewShell
browser_type: [removed]
Tree:         Android
Tests:        WebView
Package:      com.android.webview.chromium.shell
(waiting on me to remove)

[hjd, 2014-08-29 13:46:04]

> (What I think the current situation is just to be clear: 
> Target                                   browser_type           Tree     Tests
 
>     Package
> gyp:android_webview_apk                  android-webview-shell  Chrome  
> AwContents  org.chromium.android_webview.shell (this cl)
> gyp:android_webview_telemetry_shell_apk  android-webview        Chrome  
WebView
>     org.chromium.telemetry_shell
> make:WebViewShell                        [removed]              Android 
WebView
>     com.android.webview.chromium.shell (waiting on me to remove))

1)
Target:       gyp:android_webview_apk
browser_type: android-webview-shell
Tree:         Chrome
Tests:        AwContents
Package:      org.chromium.android_webview.shell
(this cl)

2)
Target:       gyp:android_webview_telemetry_shell_apk
browser_type: android-webview
Tree:         Chrome
Tests:        WebView
Package:      org.chromium.telemetry_shell

3)
Target:       make:WebViewShell
browser_type: [removed]
Tree:         Android
Tests:        WebView
Package:      com.android.webview.chromium.shell
(waiting on me to remove)

[epennerAtGoogle, 2014-08-29 19:54:44]

LGTM to land this.

However,

> 3)
> Target:       make:WebViewShell
> browser_type: [removed]
> Tree:         Android
> Tests:        WebView
> Package:      com.android.webview.chromium.shell
> (waiting on me to remove)

We shouldn't remove this in Android yet, as CTS tests might make use of it.
Please keep it until we have sorted out how the WebGL tests will work (AKA just
ping me before removing it)

[no sievers, 2014-08-29 19:56:17]

The CQ bit was checked by sievers@chromium.org

[no sievers, 2014-08-29 23:16:47]

The CQ bit was unchecked by sievers@chromium.org

[no sievers, 2014-08-29 23:16:52]

The CQ bit was checked by sievers@chromium.org

[commit-bot: I haz the power, 2014-08-29 23:18:19]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sievers@chromium.org/517573004/20001

[commit-bot: I haz the power, 2014-08-30 00:31:46]

Committed patchset #2 (id:20001) as 192ccb33a63dc797c11c02b7f01cdf49a364e234

[Primiano Tucci (use gerrit), 2014-09-01 06:56:37]

On 2014/08/30 00:31:46, I haz the power (commit-bot) wrote:
> Committed patchset #2 (id:20001) as 192ccb33a63dc797c11c02b7f01cdf49a364e234

Apologies for the delay but I was OOO friday.
I am a bit confused here. Does the upstream webview test shell
(org.chromium.android_webview.shell) that lives in Chromium support telemetry at
all?
Which kind of tests do you plan to run there? Isn't that going to be confusing?
two different targets with WebView in the name, with strongly different
performance characteristics (the upstream one doesn't have any capability of HW
rendering IIRC) ?

[no sievers, 2014-09-02 19:39:26]

On 2014/09/01 06:56:37, Primiano Tucci wrote:
> On 2014/08/30 00:31:46, I haz the power (commit-bot) wrote:
> > Committed patchset #2 (id:20001) as 192ccb33a63dc797c11c02b7f01cdf49a364e234
> 
> Apologies for the delay but I was OOO friday.
> I am a bit confused here. Does the upstream webview test shell
> (org.chromium.android_webview.shell) that lives in Chromium support telemetry
at
> all?
> Which kind of tests do you plan to run there? Isn't that going to be
confusing?
> two different targets with WebView in the name, with strongly different
> performance characteristics (the upstream one doesn't have any capability of
HW
> rendering IIRC) ?

It does since https://codereview.chromium.org/414503004/.

Mainly we need to run webgl conformance tests on the gpu waterfall since the gpu
and compositing stack is so different for WebView.

[no sievers, 2014-09-02 19:40:22]

On 2014/09/02 19:39:26, sievers wrote:
> On 2014/09/01 06:56:37, Primiano Tucci wrote:
> > On 2014/08/30 00:31:46, I haz the power (commit-bot) wrote:
> > > Committed patchset #2 (id:20001) as
192ccb33a63dc797c11c02b7f01cdf49a364e234
> > 
> > Apologies for the delay but I was OOO friday.
> > I am a bit confused here. Does the upstream webview test shell
> > (org.chromium.android_webview.shell) that lives in Chromium support
telemetry
> at
> > all?
> > Which kind of tests do you plan to run there? Isn't that going to be
> confusing?
> > two different targets with WebView in the name, with strongly different
> > performance characteristics (the upstream one doesn't have any capability of
> HW


^^ agreed that you'd have to be careful about making conclusions about
performance.
However functional testing still matters.


> > rendering IIRC) ?
> 
> It does since https://codereview.chromium.org/414503004/.
> 
> Mainly we need to run webgl conformance tests on the gpu waterfall since the
gpu
> and compositing stack is so different for WebView.

[commit-bot: I haz the power, 2014-09-10 03:12:22]

Patchset 2 (id:??) landed as
https://crrev.com/8d9f0ee4fd09b912c8aca025ccc3c8e2c653f275
Cr-Commit-Position: refs/heads/master@{#292734}