Rename chromium_testshell target to chrome_shell_apk.

Rename chromium_testshell target to chrome_shell_apk.

Final rename of ChromiumTestShell to ChromeShell.
Now the new targets to build ChromeShell and ChromeShellTest APKs will be:
chrome_shell_apk
chrome_shell_test_apk

This will better align with ContentShell and ContentShellTest APKs' targets:
content_shell_apk
content_shell_test_apk

BUG=348137
TBR=mnaganov@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=256382

[aurimas (slooooooooow), 2014-03-06 23:43:48]

Hey guys, please review changes in the following directories:

tonyg: tools/telemetry/*
nyquist: chrome/android/shell/*
boliu: android_webview/*
bulach: tools/* and build/android/*
craigdh: chrome/test/chromedriver/*
navabi: FYI

[boliu, 2014-03-07 00:00:10]

android_webview lgtm for comment only change :)

[tonyg, 2014-03-07 02:47:03]

https://chromiumcodereview.appspot.com/189133005/diff/20001/tools/telemetry/t...
File tools/telemetry/telemetry/core/backends/chrome/android_browser_finder.py
(left):

https://chromiumcodereview.appspot.com/189133005/diff/20001/tools/telemetry/t...
tools/telemetry/telemetry/core/backends/chrome/android_browser_finder.py:27: #
TODO(tonyg): rename android-chromium-testshell to android-chrome-shell
I think we'd need a master.cfg change in order to change this string,
unfortunately. If you just revert the string change (or else make a master.cfg
change), then this should be good to go.

[nyquist, 2014-03-07 05:51:58]

https://chromiumcodereview.appspot.com/189133005/diff/20001/chrome/android/sh...
File
chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellActivity.java
(right):

https://chromiumcodereview.appspot.com/189133005/diff/20001/chrome/android/sh...
chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellActivity.java:105:
mDevToolsServer = new DevToolsServer("chrome_shell");
Does this string have to match something else?

https://chromiumcodereview.appspot.com/189133005/diff/20001/chrome/chrome_and...
File chrome/chrome_android.gypi (right):

https://chromiumcodereview.appspot.com/189133005/diff/20001/chrome/chrome_and...
chrome/chrome_android.gypi:72: # chrome_shell creates a .jar as a side effect.
Any java targets
I believe this comment should read:

# chrome_shell_apk creates a .jar as a side effect. Any java targets
# that need that .jar in their classpath should depend on this target,
# chrome_shell_apk_java. Dependents of chrome_shell_apk receive its
# jar path in the variable 'apk_output_jar_path'.

And also, to make it equal to ContentShell, bonus points for adding:
"This target should only be used by targets which instrument chrome_shell_apk."

https://chromiumcodereview.appspot.com/189133005/diff/20001/chrome/chrome_and...
chrome/chrome_android.gypi:76: 'target_name': 'chrome_shell_java',
I believe this should be chrome_shell_apk_java, shouldn't it?
That would make it similar to content_shell_apk_java.

[mnaganov (inactive), 2014-03-07 09:23:17]

https://chromiumcodereview.appspot.com/189133005/diff/20001/chrome/browser/de...
File chrome/browser/devtools/devtools_adb_bridge.cc (right):

https://chromiumcodereview.appspot.com/189133005/diff/20001/chrome/browser/de...
chrome/browser/devtools/devtools_adb_bridge.cc:117:
"chrome_shell_devtools_remote",
I think you need to leave the old name and add the new one. This is desktop
code, so if you will change like this, newer desktop Chrome will not recognize
previous builds of the Test Shell.

Adding kaznacheev@ to reviewers to take a look at this.

[aurimas (slooooooooow), 2014-03-07 18:06:09]

https://chromiumcodereview.appspot.com/189133005/diff/20001/chrome/android/sh...
File
chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellActivity.java
(right):

https://chromiumcodereview.appspot.com/189133005/diff/20001/chrome/android/sh...
chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellActivity.java:105:
mDevToolsServer = new DevToolsServer("chrome_shell");
On 2014/03/07 05:51:59, nyquist wrote:
> Does this string have to match something else?

mnaganov@ said that it should be enough to change
chrome/browser/devtools/devtools_adb_bridge.cc

https://chromiumcodereview.appspot.com/189133005/diff/20001/chrome/browser/de...
File chrome/browser/devtools/devtools_adb_bridge.cc (right):

https://chromiumcodereview.appspot.com/189133005/diff/20001/chrome/browser/de...
chrome/browser/devtools/devtools_adb_bridge.cc:117:
"chrome_shell_devtools_remote",
On 2014/03/07 09:23:18, Mikhail Naganov (Cr) wrote:
> I think you need to leave the old name and add the new one. This is desktop
> code, so if you will change like this, newer desktop Chrome will not recognize
> previous builds of the Test Shell.
> 
> Adding kaznacheev@ to reviewers to take a look at this.

Do we really care to support this use case? ChromeShell is only meant for
testing anyway.

https://chromiumcodereview.appspot.com/189133005/diff/20001/chrome/chrome_and...
File chrome/chrome_android.gypi (right):

https://chromiumcodereview.appspot.com/189133005/diff/20001/chrome/chrome_and...
chrome/chrome_android.gypi:72: # chrome_shell creates a .jar as a side effect.
Any java targets
On 2014/03/07 05:51:59, nyquist wrote:
> I believe this comment should read:
> 
> # chrome_shell_apk creates a .jar as a side effect. Any java targets
> # that need that .jar in their classpath should depend on this target,
> # chrome_shell_apk_java. Dependents of chrome_shell_apk receive its
> # jar path in the variable 'apk_output_jar_path'.
> 
> And also, to make it equal to ContentShell, bonus points for adding:
> "This target should only be used by targets which instrument
chrome_shell_apk."

Done.

https://chromiumcodereview.appspot.com/189133005/diff/20001/chrome/chrome_and...
chrome/chrome_android.gypi:76: 'target_name': 'chrome_shell_java',
On 2014/03/07 05:51:59, nyquist wrote:
> I believe this should be chrome_shell_apk_java, shouldn't it?
> That would make it similar to content_shell_apk_java.

Done.

https://chromiumcodereview.appspot.com/189133005/diff/20001/tools/telemetry/t...
File tools/telemetry/telemetry/core/backends/chrome/android_browser_finder.py
(left):

https://chromiumcodereview.appspot.com/189133005/diff/20001/tools/telemetry/t...
tools/telemetry/telemetry/core/backends/chrome/android_browser_finder.py:27: #
TODO(tonyg): rename android-chromium-testshell to android-chrome-shell
On 2014/03/07 02:47:04, tonyg wrote:
> I think we'd need a master.cfg change in order to change this string,
> unfortunately. If you just revert the string change (or else make a master.cfg
> change), then this should be good to go.

I reverted this string change.

[craigdh, 2014-03-07 18:57:27]

/build/android/pylib/

and

/chrome/test/chromedriver/

lgtm.

[nyquist, 2014-03-07 23:58:27]

//chrome/android/ lgtm

[aurimas (slooooooooow), 2014-03-08 00:03:48]

tonyq: is tools/telemery/ good to go?

bulach: ping?

[tonyg, 2014-03-08 17:23:24]

telemetry lgtm

[mnaganov (inactive), 2014-03-10 08:37:43]

https://chromiumcodereview.appspot.com/189133005/diff/20001/chrome/browser/de...
File chrome/browser/devtools/devtools_adb_bridge.cc (right):

https://chromiumcodereview.appspot.com/189133005/diff/20001/chrome/browser/de...
chrome/browser/devtools/devtools_adb_bridge.cc:117:
"chrome_shell_devtools_remote",
On 2014/03/07 18:06:09, aurimas wrote:
> On 2014/03/07 09:23:18, Mikhail Naganov (Cr) wrote:
> > I think you need to leave the old name and add the new one. This is desktop
> > code, so if you will change like this, newer desktop Chrome will not
recognize
> > previous builds of the Test Shell.
> > 
> > Adding kaznacheev@ to reviewers to take a look at this.
> 
> Do we really care to support this use case? ChromeShell is only meant for
> testing anyway.

Hmm, maybe not. OK, let's avoid polluting this list.

[bulach, 2014-03-10 09:57:57]

lgtm, thanks!!

as for telemetry + buildbot, I think for a change it's slightly simpler than a
master restart ;)

https://code.google.com/p/chromium/codesearch#chromium/tools/build/scripts/sl...

no need for bot restart..

of course, it's a N-sided patch, so need to tweak telemetry.py (in the buildbot
side), wait for all the bots to catch up, then remove in tools/telemetry in the
chromium side.

[Vladislav Kaznacheev, 2014-03-11 11:23:04]

On 2014/03/07 09:23:17, Mikhail Naganov (Cr) wrote:
>
https://chromiumcodereview.appspot.com/189133005/diff/20001/chrome/browser/de...
> File chrome/browser/devtools/devtools_adb_bridge.cc (right):
> 
>
https://chromiumcodereview.appspot.com/189133005/diff/20001/chrome/browser/de...
> chrome/browser/devtools/devtools_adb_bridge.cc:117:
> "chrome_shell_devtools_remote",
> I think you need to leave the old name and add the new one. This is desktop
> code, so if you will change like this, newer desktop Chrome will not recognize
> previous builds of the Test Shell.
> 
> Adding kaznacheev@ to reviewers to take a look at this.

I confirm that what mnaganov@ said could be a problem, but it could be fixed by
building and installing a newer Test Shell apk.
I am not really familiar with the Test Shell usage, so I leave it to the patch
author to decide whether to leave the change as it is or add an extra line.

[aurimas (slooooooooow), 2014-03-11 16:20:45]

+kerz for chrome/tools/build/android/FILES.cfg

https://chromiumcodereview.appspot.com/189133005/diff/20001/chrome/browser/de...
File chrome/browser/devtools/devtools_adb_bridge.cc (right):

https://chromiumcodereview.appspot.com/189133005/diff/20001/chrome/browser/de...
chrome/browser/devtools/devtools_adb_bridge.cc:117:
"chrome_shell_devtools_remote",
On 2014/03/10 08:37:44, Mikhail Naganov (Cr) wrote:
> On 2014/03/07 18:06:09, aurimas wrote:
> > On 2014/03/07 09:23:18, Mikhail Naganov (Cr) wrote:
> > > I think you need to leave the old name and add the new one. This is
desktop
> > > code, so if you will change like this, newer desktop Chrome will not
> recognize
> > > previous builds of the Test Shell.
> > > 
> > > Adding kaznacheev@ to reviewers to take a look at this.
> > 
> > Do we really care to support this use case? ChromeShell is only meant for
> > testing anyway.
> 
> Hmm, maybe not. OK, let's avoid polluting this list.

I removed the old chromium_testshell from the list.

[kerz_google, 2014-03-11 16:27:42]

lgtm

[aurimas (slooooooooow), 2014-03-11 18:31:18]

The CQ bit was checked by aurimas@chromium.org

[commit-bot: I haz the power, 2014-03-11 19:08:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aurimas@chromium.org/189133005/70001

[commit-bot: I haz the power, 2014-03-12 01:31:51]

Change committed as 256382