Fix(espresso): Fix #2349 #2359
Fix(espresso): Fix #2349 #2359
Conversation
|
Not sure why copybara is dying |
|
@brettchabot Are you able to fetch the error from I don't seem to be able to do so, this blocks me from making the necessary changes for copybara to be happy |
|
apparently I just needed to rebase on top of the two new commits from this repo, weird given there was no conflict |
|
@brettchabot Can you review the PR ? Or someone else, I don't know |
| @@ -49,6 +50,11 @@ public static void waitForConfigurationChangesOnActivity( | |||
| if (Build.VERSION.SDK_INT >= 24 && currentActivity.isInMultiWindowMode()) { | |||
| return; | |||
| } | |||
| // If the application is running activities in different processes, activities that aren't | |||
| // on the main process may have a different orientation | |||
| if (Build.VERSION.SDK_INT >= 28 && !currentActivity.getApplicationInfo().processName.equals(Application.getProcessName())) { | |||
There was a problem hiding this comment.
I'm confused how this statement could ever be true. I'm not familiar with this class but I don't see how it could be called with an Activity from a different process.
Do you have a test that can both repro the problem you are seeing and verify this fixes it?
There was a problem hiding this comment.
@brettchabot Yes, as mentioned in #2349, there is this repository: https://github.com/Mathias-Boulay/expresso-issue
Branch main: the issue will trigger
Branch fix-included (newly added for your convenience): the issue won't trigger, as it uses the fixed library.
You should be able to use Android Studio Meerkat for both branches to run the only test in the project
There was a problem hiding this comment.
OK I think I understand how this fix could work. IIUC currentActivity should always be running in the current process, but it could be possible that the Application's default process is different.
However, doesn't this change run the risk of introducing flakiness? With this change it looks like waitForConfigurationChangesOnActivity will always return immediately if the currentActivity is running in a secondary process.
I suppose it is no different than when the activity is in multi window mode....
I'll import this change internally and run a few more tests. Sorry for the delay in getting this reviewed
| @@ -49,6 +50,11 @@ public static void waitForConfigurationChangesOnActivity( | |||
| if (Build.VERSION.SDK_INT >= 24 && currentActivity.isInMultiWindowMode()) { | |||
| return; | |||
| } | |||
| // If the application is running activities in different processes, activities that aren't | |||
| // on the main process may have a different orientation | |||
| if (Build.VERSION.SDK_INT >= 28 && !currentActivity.getApplicationInfo().processName.equals(Application.getProcessName())) { | |||
There was a problem hiding this comment.
currentActivity.getApplicationInfo().processName can be null in Robolectric environments. Consider using Objects.equals
Overview
This pull request fixes an issue I brought up a little while ago (#2349) with multi-process + multi activity application that use different rotations between the activities.
Proposed Changes
Since the processes are different, an early return is added to allow for activities on other processes to have a rotation different from the main process.
Note: the fix only works on API 28 or later due to limitations from the public API.