Fix "Class resolved by unexpected DEX" in uiautomator tests

Fix "Class resolved by unexpected DEX" in uiautomator tests

Attemping to run uiautomator tests on a Nexus fails with the following error:

W/dalvikvm(32338): Class resolved by unexpected DEX: Lorg/chromium/chrome/shell/uiautomator/DummyTest;(0x416544d0):0x5b738000 ref [Lcom/android/uiautomator/testrunner/              UiAutomatorTestCase;] Lcom/android/uiautomator/testrunner/UiAutomatorTestCase;(0x416544d0):0x5b6dc000
W/dalvikvm(32338): (Lorg/chromium/chrome/shell/uiautomator/DummyTest; had used a different Lcom/android/uiautomator/testrunner/UiAutomatorTestCase; during pre-verification)
W/dalvikvm(32338): Unable to resolve superclass of Lorg/chromium/chrome/shell/uiautomator/DummyTest; (96)
W/dalvikvm(32338): Link of class 'Lorg/chromium/chrome/shell/uiautomator/DummyTest;' failed
D/AndroidRuntime(32338): Shutting down VM
W/dalvikvm(32338): threadid=1: thread exiting with uncaught exception (group=0x4164abc0)
E/UiAutomatorTestRunner(32338): uncaught exception
E/UiAutomatorTestRunner(32338): java.lang.IllegalAccessError: Class ref in pre-verified class resolved to unexpected implementation

This error occurs because uiautomator.jar is built into
out/Debug/lib.java/chrome_shell_uiautomator_tests.dex.jar. This results in two
different implementations of uiautomator classes on the device hence the
dalvik error.

The 'chrome_shell_uiautomator_tests' gyp target uses the uiautomator_test.gypi
include. uiautomator.gypi dexes all dependent jars into a single .dex.jar.
Since chrome_shell_uiautomator_tests has a transitive dependence,
through chrome_shell_uiautomator_tests_java, on uiautomator.gyp,
the uiautomator jar used for compilation will be built into its output.

If we ignore chrome_shell_uiautomator_tests and instead just build
chrome_shell_uiautomator_tests_java we can successfully run uiautomator tests.
This gyp target already generates dexed jars through java.gypi (which calls
dex_action.gypi).

This commit removes uiautomator_test.gypi and renames
chrome_shell_uiautomator_tests_java to chrome_shell_uiautomator_tests.

BUG=434080
Committed: https://crrev.com/57dc71e6bf75f6f249c941a70c9139e1316516f9
Cr-Commit-Position: refs/heads/master@{#306315}

[davileen, 2014-11-17 23:36:36]

davileen@amazon.com changed reviewers:
+ dtrainor@chromium.org

[davileen, 2014-11-17 23:48:03]

On 2014/11/17 23:36:36, davileen wrote:
> mailto:davileen@amazon.com changed reviewers:
> + mailto:dtrainor@chromium.org

Hi David,

Here's a proposed fix for
https://code.google.com/p/chromium/issues/detail?id=434080

Please let me know what you think.

Thanks,
David.

[David Trainor- moved to gerrit, 2014-11-18 19:31:39]

dtrainor@chromium.org changed reviewers:
+ cjhopman@chromium.org

[David Trainor- moved to gerrit, 2014-11-18 19:31:40]

+cjhopman.  He knows this area much better than me.

[cjhopman, 2014-11-18 20:38:22]

On 2014/11/17 23:48:03, davileen wrote:
> On 2014/11/17 23:36:36, davileen wrote:
> > mailto:davileen@amazon.com changed reviewers:
> > + mailto:dtrainor@chromium.org
> 
> Hi David,
> 
> Here's a proposed fix for
> https://code.google.com/p/chromium/issues/detail?id=434080
> 
> Please let me know what you think.
> 
> Thanks,
> David.

I don't think that this will work. android_library/java.gypi targets don't
create a single monolithic dex file, they only create a dex for their own java
files.

I would like to know how the normal Android build system builds uiautomator test
targets.

Can we just use the one large monolithic dex that includes uiautomator (and not
the uiautomator standalone one)?

[davileen, 2014-11-18 23:46:27]

On 2014/11/18 20:38:22, cjhopman wrote:
> On 2014/11/17 23:48:03, davileen wrote:
> > On 2014/11/17 23:36:36, davileen wrote:
> > > mailto:davileen@amazon.com changed reviewers:
> > > + mailto:dtrainor@chromium.org
> > 
> > Hi David,
> > 
> > Here's a proposed fix for
> > https://code.google.com/p/chromium/issues/detail?id=434080
> > 
> > Please let me know what you think.
> > 
> > Thanks,
> > David.
> 
> I don't think that this will work. android_library/java.gypi targets don't
> create a single monolithic dex file, they only create a dex for their own java
> files.
> 
> I would like to know how the normal Android build system builds uiautomator
test
> targets.
> 
> Can we just use the one large monolithic dex that includes uiautomator (and
not
> the uiautomator standalone one)?

Hi cjhopman,

Thanks for the review!

You're right. This change goes too far and won't include other dependencies in
the dex. 

There's nothing special about building uiautomator test targets. The problem is
that uiautomator.jar is being treated as an actual dependency (like how we'd
treat base_java.jar for example) instead of treating it more like android.jar.
This means that the dexed uiautomator is being included in the monolithic dex.

I have two much simpler solutions now:

1. In chrome_tests.gypi in the target chrome_shell_uiautomator_tests_java I can
add:

'input_jars_paths': [ '<(android_sdk)/uiautomator.jar' ],
and remove the dependency on
'../third_party/android_tools/android_tools.gyp:uiautomator_jar'

2. Alternatively in build/java.gypi

'uiautomator_jar': '<(android_sdk)/uiautomator.jar',
'input_jars_paths': [ '<(android_jar)', '<(uiautomator_jar)' ],

and again remove the dependency on uiautomator_jar.

We can then discard the rest of this CL.

I like the change to build/java.gypi since it's transparent and puts
uiautomator.jar on the same footing as android.jar.

What do you think?

David.

[cjhopman, 2014-11-24 01:29:32]

Wow, sorry for the delay here. I drafted a reply but failed to send it...



On 2014/11/18 23:46:27, davileen wrote:
> On 2014/11/18 20:38:22, cjhopman wrote:
> > On 2014/11/17 23:48:03, davileen wrote:
> > > On 2014/11/17 23:36:36, davileen wrote:
> > > > mailto:davileen@amazon.com changed reviewers:
> > > > + mailto:dtrainor@chromium.org
> > > 
> > > Hi David,
> > > 
> > > Here's a proposed fix for
> > > https://code.google.com/p/chromium/issues/detail?id=434080
> > > 
> > > Please let me know what you think.
> > > 
> > > Thanks,
> > > David.
> > 
> > I don't think that this will work. android_library/java.gypi targets don't
> > create a single monolithic dex file, they only create a dex for their own
java
> > files.
> > 
> > I would like to know how the normal Android build system builds uiautomator
> test
> > targets.
> > 
> > Can we just use the one large monolithic dex that includes uiautomator (and
> not
> > the uiautomator standalone one)?
> 
> Hi cjhopman,
> 
> Thanks for the review!
> 
> You're right. This change goes too far and won't include other dependencies in
> the dex. 
> 
> There's nothing special about building uiautomator test targets. The problem
is
> that uiautomator.jar is being treated as an actual dependency (like how we'd
> treat base_java.jar for example) instead of treating it more like android.jar.
> This means that the dexed uiautomator is being included in the monolithic dex.

Great, thanks.

> 
> I have two much simpler solutions now:
> 
> 1. In chrome_tests.gypi in the target chrome_shell_uiautomator_tests_java I
can
> add:
> 
> 'input_jars_paths': [ '<(android_sdk)/uiautomator.jar' ],
> and remove the dependency on
> '../third_party/android_tools/android_tools.gyp:uiautomator_jar'

I would avoid this... input_jars_paths is sort of an implementation detail of
our .gypi files that shouldn't be used outside of them.

> 
> 2. Alternatively in build/java.gypi
> 
> 'uiautomator_jar': '<(android_sdk)/uiautomator.jar',
> 'input_jars_paths': [ '<(android_jar)', '<(uiautomator_jar)' ],
> 
> and again remove the dependency on uiautomator_jar.
> 
> We can then discard the rest of this CL.
> 
> I like the change to build/java.gypi since it's transparent and puts
> uiautomator.jar on the same footing as android.jar.

This would work, but I would like us to come up with something that prevents
non-uiautomator targets from referring to stuff in that jar.

How about adding a flag to build/java_prebuilt.gypi that makes it so that the
jar is only used at compile time and not runtime (i.e. not added to the dex
files)? To implement this in java_prebuilt.gypi, just don't add the dexed path
to library_dexed_jars_paths (and don't need to do the dexing at all, then). I'd
use the name "neverlink" for this flag (that name matches another build system
that many chrome developers may be familiar with).

> 
> What do you think?
> 
> David.

[davileen, 2014-11-27 01:47:10]

> How about adding a flag to build/java_prebuilt.gypi that makes it so that the
> jar is only used at compile time and not runtime (i.e. not added to the dex
> files)? To implement this in java_prebuilt.gypi, just don't add the dexed path
> to library_dexed_jars_paths (and don't need to do the dexing at all, then).
I'd
> use the name "neverlink" for this flag (that name matches another build system
> that many chrome developers may be familiar with).

Thanks for the reply! It sounds good. I'll post a new diff soon.

It looks like I'll have to make a change to
third_party/android_tools/android_tools.gyp to set the neverlink variable. So
I'll have to make a commit to that repository and update DEPS. I'm not sure how
to go about making a change in android_tools though. I see the wiki
http://www.chromium.org/developers/contributing-code has some info for blink and
devtools but not android_tools. Do you have any guidance?

Thanks,
David.

[davileen, 2014-11-29 00:21:47]

On 2014/11/27 01:47:10, davileen wrote:
> > How about adding a flag to build/java_prebuilt.gypi that makes it so that
the
> > jar is only used at compile time and not runtime (i.e. not added to the dex
> > files)? To implement this in java_prebuilt.gypi, just don't add the dexed
path
> > to library_dexed_jars_paths (and don't need to do the dexing at all, then).
> I'd
> > use the name "neverlink" for this flag (that name matches another build
system
> > that many chrome developers may be familiar with).
> 
> Thanks for the reply! It sounds good. I'll post a new diff soon.
> 
> It looks like I'll have to make a change to
> third_party/android_tools/android_tools.gyp to set the neverlink variable. So
> I'll have to make a commit to that repository and update DEPS. I'm not sure
how
> to go about making a change in android_tools though. I see the wiki
> http://www.chromium.org/developers/contributing-code has some info for blink
and
> devtools but not android_tools. Do you have any guidance?
> 
> Thanks,
> David.

Added a neverlink variable. I think this CL will need to land before I use the
variable in android_tools.gyp. I still need some guidance on how to make that
change if you could point me in the right direction. 

When I make the change in android_tools I'll bump the DEPS in the main repo and
the issue should be resolved.

Thanks,
David.

[cjhopman, 2014-12-01 21:41:22]

lgtm

[cjhopman, 2014-12-01 22:22:09]

On 2014/12/01 21:41:22, cjhopman wrote:
> lgtm

Also, for making changes to android_tools. Here's our internal instructions (I
think they'll work for you, if not I'll make the change myself):

(Note, you shouldn't have to re-clone the repo, just do this in
third_party/android_tools)

Make sure your SSH public key has been added to https://gerrit.chromium.org/
Checkout the android_tools repo
git clone ssh://<username>@gerrit.chromium.org:29418/android_tools.git
# Set user.name and user.email if you didn't set them globally
git config user.name "Your Name"
git config user.email "your.id@chromium.org"
Make your change and commit normally
<make your changes>
git commit -a
Push the change for review (make sure you have your @chromium.org account set
for git as described above)
git push ssh://<username>@gerrit.chromium.org:29418/android_tools.git
HEAD:refs/for/master
Add reviewers and get LGTM's and add navabi or rmcilroy.
If you need to upload an updated patch use the same command above, but make sure
to add "Change-Id: ********" to your local commit message where that change-id
is taken from the gerrit page for this issue.
Once you have your LGTM, notify aurimas, navabi or rmcilroy to commit the change
(this last part needs to change to let people commit their own changes to
android_tools).
Don't forget to roll Chromium to the new commit in android_tools repository by
modifying the DEPS file in the src/ directory, like this

[davileen, 2014-12-01 22:40:14]

The CQ bit was checked by davileen@amazon.com

[commit-bot: I haz the power, 2014-12-01 22:41:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/734793002/20001

[commit-bot: I haz the power, 2014-12-02 00:28:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-02 00:28:37]

Try jobs failed on following builders:
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[davileen, 2014-12-02 00:38:34]

The CQ bit was checked by davileen@amazon.com

[commit-bot: I haz the power, 2014-12-02 00:39:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/734793002/20001

[commit-bot: I haz the power, 2014-12-02 01:31:30]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-12-02 01:33:14]

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

[davileen, 2014-12-08 18:45:38]

On 2014/12/01 22:22:09, cjhopman wrote:
> On 2014/12/01 21:41:22, cjhopman wrote:
> > lgtm
> 
> Also, for making changes to android_tools. Here's our internal instructions (I
> think they'll work for you, if not I'll make the change myself):
> 
> (Note, you shouldn't have to re-clone the repo, just do this in
> third_party/android_tools)
> 
> Make sure your SSH public key has been added to https://gerrit.chromium.org/
> Checkout the android_tools repo
> git clone ssh://<username>@gerrit.chromium.org:29418/android_tools.git
> # Set user.name and user.email if you didn't set them globally
> git config user.name "Your Name"
> git config user.email mailto:"your.id@chromium.org"
> Make your change and commit normally
> <make your changes>
> git commit -a
> Push the change for review (make sure you have your @chromium.org account set
> for git as described above)
> git push ssh://<username>@gerrit.chromium.org:29418/android_tools.git
> HEAD:refs/for/master
> Add reviewers and get LGTM's and add navabi or rmcilroy.
> If you need to upload an updated patch use the same command above, but make
sure
> to add "Change-Id: ********" to your local commit message where that change-id
> is taken from the gerrit page for this issue.
> Once you have your LGTM, notify aurimas, navabi or rmcilroy to commit the
change
> (this last part needs to change to let people commit their own changes to
> android_tools).
> Don't forget to roll Chromium to the new commit in android_tools repository by
> modifying the DEPS file in the src/ directory, like this

Hi,

I didn't manage to get these steps to work, and posting a bug to infra didn't
help (https://code.google.com/p/chromium/issues/detail?id=438457), so I'd love
to take you up on your offer of making the change yourself.

Thanks,
David