Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update target SDK version to 29 #106

Closed
garethbowen opened this issue May 26, 2020 · 29 comments
Closed

Update target SDK version to 29 #106

garethbowen opened this issue May 26, 2020 · 29 comments
Assignees
Labels
Priority: 1 - High Blocking the next release Type: Technical issue Improve something that users won't notice

Comments

@garethbowen
Copy link
Contributor

There are upcoming deadlines regarding target SDK version for updating or publishing new apps.

August 3, 2020: new apps must target at least Android 10 (API level 29).
November 2, 2020: all app updates must target at least Android 10 (API level 29).

@garethbowen garethbowen added Priority: 2 - Medium Normal priority Type: Technical issue Improve something that users won't notice labels May 26, 2020
@njogz njogz self-assigned this Aug 3, 2020
@njogz njogz linked a pull request Aug 5, 2020 that will close this issue
@njogz njogz removed a link to a pull request Aug 5, 2020
@garethbowen
Copy link
Contributor Author

Ready for AT on 106-update-target-sdk-version

AT instructions:

  • test multiple devices, and Android 10 if you can.
  • also there's a comment in the code that...

When upgrading targetSdkVersion, check that the app menu still works on newer devices.

@ngaruko
Copy link
Contributor

ngaruko commented Aug 19, 2020

App (branch) crashes with the following errors (on Android 10; seems to work on android 8) - and master also seems to work on android 10.

2020-08-19 16:20:28.066 25408-27276/org.medicmobile.webapp.mobile E/libc: Access denied finding property "net.dns1"
2020-08-19 16:20:28.066 25408-27276/org.medicmobile.webapp.mobile E/libc: Access denied finding property "net.dns2"
2020-08-19 16:20:28.102 25408-25408/org.medicmobile.webapp.mobile E/chromium: [ERROR:xwalk_platform_notification_service.cc(143)] Not implemented reached in virtual bool xwalk::XWalkPlatformNotificationService::GetDisplayedPersistentNotifications(content::BrowserContext*, std::__1::set<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >*)
2020-08-19 16:20:28.111 25408-25408/org.medicmobile.webapp.mobile E/chromium: [ERROR:xwalk_browser_context.cc(88)] Failed to read preference, error num: 0
2020-08-19 16:20:28.460 25408-25408/org.medicmobile.webapp.mobile E/chromium: [ERROR:compositor_impl_android.cc(585)] Failed to init OutputSurface for compositor.
2020-08-19 16:20:28.460 25408-25408/org.medicmobile.webapp.mobile E/chromium: [ERROR:compositor_impl_android.cc(585)] Failed to init OutputSurface for compositor.
2020-08-19 16:20:28.473 25408-25408/org.medicmobile.webapp.mobile A/chromium: [FATAL:compositor_impl_android.cc(586)] Too many context creation failures. Giving up... 
  
    --------- beginning of crash
2020-08-19 16:20:28.474 25408-25408/org.medicmobile.webapp.mobile A/libc: Fatal signal 6 (SIGABRT), code -1 (SI_QUEUE) in tid 25408 (e.webapp.mobile), pid 25408 (e.webapp.mobile)

image

Not sure if it is related to #65 @njogz ?

@njogz
Copy link
Contributor

njogz commented Aug 19, 2020

It seems to be different. Looking into it.

@garethbowen
Copy link
Contributor Author

@njogz has raised a new issue for Android 10 support, but we still need to update the target sdk version before November 2nd to continue to be able to publish to the app store, so I'm putting this back in for AT.

@ngaruko Other than Android 10 was the rest of the testing successful?

@garethbowen
Copy link
Contributor Author

This comment says their issue was resolved by fixing a misconfiguration of the network on the phone which makes Access denied finding property "net.dns1" potentially relevant.

@ngaruko Can you provide details on how you tested this? Was it emulated or on an actual device? I assume you built and deployed it locally via USB?

We need to replicate this on another device to test if it's specific to your phone...

@garethbowen
Copy link
Contributor Author

@njogz How did you get on with this? Is it possible to fix the bug without dropping crosswalk for Android 10?

@njogz
Copy link
Contributor

njogz commented Sep 8, 2020

@garethbowen I haven't been able to find a solution that does not involve dropping crosswalk. I looked into required permissions introduced by android 10 as we had discussed but didn't have much luck with that either.

@ngaruko
Copy link
Contributor

ngaruko commented Sep 8, 2020

All logs (verbose):

logs.log

@newtewt
Copy link
Contributor

newtewt commented Sep 15, 2020

Pushing this back to In Progress as there is still investigation going on.

@makombe
Copy link

makombe commented Nov 12, 2020

I am not able to publish to the app store because of the target SDK version to 29. The app crashes. Has this been sorted out?

@newtewt
Copy link
Contributor

newtewt commented Nov 12, 2020

@makombe it's in progress to be fixed right now. @garethbowen is doing the development on it.

@makombe
Copy link

makombe commented Nov 12, 2020

@newtewt Thanks for the feedback, eagerly waiting for the fix.

@garethbowen
Copy link
Contributor Author

Ready for AT in 106-webview-for-android-10.

This branch adds a new android "flavour" for Android 10 and above which uses webview instead of crosswalk. Test that all the old supported features work, in particular: camera, GPS, mRDT, back button, etc.

Also check performance against the old version to ensure that it's approximately the same.

In addition this branch also includes a migration to copy data (pouchdb and cookies) from the crosswalk dir to the webview dir. It does not copy the service worker cache so you will need to have a connection when you first load the app to download the static files again. To test the migration...

  1. Install the latest version of the app either from master or the Play Store on a phone running Android 10 or later
  2. Log in and sync all data
  3. Install the branch version of the app
  4. Start the app again. At this point the migration will run and the app may automatically reload. There is a delay on writing the files so you will see the login screen.
  5. Wait a minute or so and restart the app again, after which the migrated files will be found and you will be logged in automatically and all your data will be available without having to do an initial replication again.

It is hoped to roll the webview version out to Android versions 5 and above at a later date at which point we may need to put more work into making the migration smooth and user friendly, but we are in a hurry to get this step out in order to meet Play Store requirements so we can keep publishing the app.

@ngaruko ngaruko self-assigned this Nov 15, 2020
@ngaruko
Copy link
Contributor

ngaruko commented Nov 17, 2020

Testing on 106-webview-for-android-10 branch. A few remarks, none of which is a blocker:
1 - does not load on lower versions -which is expected, just has to be documented/communicated :
Skipping device 'Redmi 6A - 9' for 'medic-android:unbranded-webview-debug': minSdkVersion [29] > deviceApiLevel [28]

2- upgradding from Google Play install to branch
with android studio
image
But in the terminal with make command, task fails

Task :installUnbrandedWebviewDebug FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':installUnbrandedWebviewDebug'.
> java.util.concurrent.ExecutionException: com.android.builder.testing.api.DeviceException: com.android.ddmlib.InstallException: INSTALL_FAILED_VERSION_DOWNGRADE

Also expected since we are loading the app from different source and it is not going to be a production process - but wondering if we could give the delete existing app option or --force flag to the make command.
3 - There are probably lingering pieces of code that still call crosswalk, and throws error in the terminal - > maybe this one needs a closer look @garethbowen ?

11-17 14:51:37.530 27511 27511 D MedicMobile: org.medicmobile.webapp.mobile.XWalkMigration :: Set up Crosswalk migration, false, org.medicmobile.webapp.mobile.EmbeddedBrowserActivity@53bcb36, android.app.Application@108292f
11-17 14:51:37.530 27511 27511 D MedicMobile: org.medicmobile.webapp.mobile.XWalkMigration :: Running Crosswalk migration
11-17 14:51:37.531 27511 27511 D MedicMobile: org.medicmobile.webapp.mobile.XWalkMigration :: exists '/data/user/0/org.medicmobile.webapp.mobile/app_xwalkcore/Default: false
11-17 14:51:37.532 27511 27511 D MedicMobile: org.medicmobile.webapp.mobile.XWalkMigration :: Crosswalk directory not found: /data/user/0/org.medicmobile.webapp.mobile/files
11-17 14:51:37.534 27511 27511 D MedicMobile: org.medicmobile.webapp.mobile.XWalkMigration :: exists '/storage/emulated/0/Android/data/org.medicmobile.webapp.mobile/app_xwalkcore/Default: false
11-17 14:51:37.535 27511 27511 D MedicMobile: org.medicmobile.webapp.mobile.XWalkMigration :: Crosswalk directory not found: /storage/emulated/0/Android/data/org.medicmobile.webapp.mobile/files
11-17 14:51:37.535 27511 27511 D MedicMobile: org.medicmobile.webapp.mobile.XWalkMigration :: Crosswalk directory not found - skipping migration``` 

@abbyad
Copy link
Contributor

abbyad commented Nov 17, 2020

Re the INSTALL_FAILED_VERSION_DOWNGRADE error... what is the the version code of the apk you are installing? If you are building locally it is likely 1, with a version name of SNAPSHOT (it is set properly when building via Travis). For the sake of the first tests you may want to manually set it to a number above what you have in the app on your device.

@ngaruko
Copy link
Contributor

ngaruko commented Nov 17, 2020

Then Back button sometimes does not behave as expected, but since it happens also on master, I have created a different issue for that. (#126 )

@antonykhaemba
Copy link
Member

Hi @garethbowen and @njogz
When do we expect the fix to be ready? Safari Doctors project are going live in the next 8 days and I wanted to confirm if we would manage to publish the app by then.

cc @derickl and @billwambua

@ngaruko
Copy link
Contributor

ngaruko commented Nov 17, 2020

@antonykhaemba
Testing is finished now. I would encourage your team to give it a quick sanity check once published, especially around migration/replication on upgrading from previous versions (for Android 10 and above).
@garethbowen AT done, the above noted issues are either known (back button) or non-blocker anyhow. We also need to update our documentaion (if we haven't yet) for this, as the new version does not load on Android 9 and below.

@garethbowen
Copy link
Contributor Author

There are probably lingering pieces of code that still call crosswalk, and throws error in the terminal - > maybe this one needs a closer look

That code isn't crosswalk, it's the crosswalk migration which runs every time the app starts. After the migration has run it fails to find files to migrate and does nothing. The log output is perhaps unnecessary but it shows it's working as expected.

@newtewt
Copy link
Contributor

newtewt commented Nov 17, 2020

@garethbowen that script is used to retain the data when going from crosswalk to the newer webview version right? @ngaruko did you confirm that upgrading doesn't wipe the data? I thought side loading or installing from the gradle wipes everything so I'm a little concerned at how we would test that.

@garethbowen garethbowen added Priority: 1 - High Blocking the next release and removed Priority: 2 - Medium Normal priority labels Nov 17, 2020
@garethbowen
Copy link
Contributor Author

Yes the script moves cookies (session) and indexeddb (pouchdb) data from the crosswalk dir to the webview dir. I tested during development by side loading from gradle and after reloading the app a couple of times I was automatically logged in and skipped the initial replication step. This proved to me that both cookies and indexeddb were being migrated correctly (albeit with some delay). This would not have worked if the gradle install wiped everything and I'm not sure why that would be happening for you.

I'll try and get travis to push an alpha of the unbranded apk to the play store ASAP to make testing easier and more production-like.

@newtewt
Copy link
Contributor

newtewt commented Nov 18, 2020

@garethbowen I think we need a way to handle connection errors presented by chrome. I have to close the app to get rid of the error and then ensure I am connected to data.

I am able to get the connection error message by

  1. Installing the main branch.
  2. logging in to my ngrok instance with offline user.
  3. Disable wifi.
  4. Install the webview version and launch.
  5. Brings you to the error shown in the screenshot below. You have to close the app and relaunch. Back button just navigates out of the app.

image

@garethbowen
Copy link
Contributor Author

We used to detect that and render a error message with a "retry" button. I'll investigate what changed and try and reimplement it.

@garethbowen
Copy link
Contributor Author

@newtewt I've fixed a regression where we added a slightly nicer error message and a retry button at the bottom of the error page. I think this is broken in the Crosswalk version too so I'm not sure when this regression occurred. I'm still working on getting the alpha build to be published, so to test you can either wait for that, or build the apk from source.

@garethbowen
Copy link
Contributor Author

@antonykhaemba We're putting the final touches on testing now - so far so good. Have you got an exact date you need this release by? Would you be interested in a pre-release apk so you can do some testing of your own?

@antonykhaemba
Copy link
Member

The Safari Doctors project is supposed to go live on 1st of December 2020. Yes I would be interested in testing the pre-release apk? @garethbowen

cc @billwambua

@garethbowen
Copy link
Contributor Author

@antonykhaemba Great. I expect to have a release out some time this week and will let you know as soon as it lands. If you want to have a go before that you can create your flavour branch from the 106-webview-for-android-10 branch and then tag it like normal. FYI there is a less than ideal experience for users when upgrading from an older version but as this is a new project this shouldn't affect you - I'll document this properly in the release notes when the release is ready to go.

@mrjones-plip
Copy link
Collaborator

I've finished all my testing that I can and put my results on the spreadsheet.

I guess my only concern is with @garethbowen's comment (below) means we can't fully test the play store upgrade on Android 10 😞

I can't publish 0.5.x (or earlier) to the Play Store because of the exact sdk version limit that this branch is trying to fix. What that means is it's impossible to set up a new app on the Play Store to accurately test it out."
- slack

However, @garethbowen has a LOT more experience than me with this, so I'll defer to him!!

garethbowen added a commit that referenced this issue Nov 26, 2020

Verified

This commit was created on github.com and signed with GitHub’s verified signature. The key has expired.
This bumps the target SDK version for Play Store compliance.
It also introduces a webview based flavor for Android 10+ devices with a data migration step.
The build has been modified so it doesn't publish each brand's app.
It also adds a "gamma" brand for internal testing.

#106
@garethbowen
Copy link
Contributor Author

This has been released in v0.6.0

derickl pushed a commit that referenced this issue Jan 21, 2021
This bumps the target SDK version for Play Store compliance.
It also introduces a webview based flavor for Android 10+ devices with a data migration step.
The build has been modified so it doesn't publish each brand's app.
It also adds a "gamma" brand for internal testing.

#106
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: 1 - High Blocking the next release Type: Technical issue Improve something that users won't notice
Projects
None yet
Development

No branches or pull requests

8 participants