Emergencies aside, there is the much more frequent case where a newly enrolled device with an old FSI auto-updates past the current required_platform_version. It will then not get any further app updates until the required_platform_version is updated.

The other use case is that we have the same packaged app being deployed to different QA devices that track beta as well as stable chromeos. The manifest can't simultaneously specify two versions. Hence app updates would not apply on the beta machines.

We need to be able to rely on app pushes being downloaded and applied regardless of version.

https://codereview.chromium.org/2615633010/

From what I can see here: https://support.google.com/chrome/a/answer/3316168?hl=en

.) If the required_platform_version set to lower than the current Chrome OS version on 'install' - we go ahead and still install the app.
.) If the required_platform_version set to lower than the current Chrome OS version on 'update' - we don't update the app.

This seems a bit inconsistent. I am curious to know why we went with this flow.

There was a discussion on this but I failed to find it in doc. Think this is the reasoning when we come up with the flow:

"required_platform_version" is created as a way to make the app run on a tested platform so that chromeos AU does not accidentally break the app. We want to keep that promise as much as we could. However, a fresh device could be on an incompliant version when it gets enrolled. The exception of always allowing first install is made so that freshly enrolled device will always have the app. And we respect required_platform_version on app update.

Gotcha. Thanks for the explanation!

This is the first I've learned of the required_platform_version key, so sorry for the simplistic questions.  But from the original implementation, it seems like this wasn't the desired behavior.

"This feature is somewhat the opposite of minimum_chrome_version, like a maximum_chrome_version. That is, when an auto launched kiosk app has the key, we want to limit Chrome's version to never go beyond that version." [1]

Why the change here?  If apps are okay with having arbitrary versions of chrome, why do they specify the required chrome version?

Also, it looks like asargent@ raised some concerns that were going to be answered off-thread in that original review; could you forward any of those discussions to me off-thread?

[1] https://codereview.chromium.org/1595483004/

"always_update" is provided as a way for app developers to indicate that the apps should always be updated regardless of the underlying OS version. This is needed because the current impl defers app update until the underlying OS becomes compliant with the platform version in app manifest. This could be undesirable and too slow in certain scenarios (e.g. critical security fix or emergent app update).

However, "always_update" key would not change required platform version restriction imposed on OS AU. OS update still respect the required platform version and "always_update" only affects the app update.

From #5:

.) If the required_platform_version set to lower than the current Chrome OS version on 'install' - we go ahead and still install the app.
.) If the required_platform_version set to lower than the current Chrome OS version on 'update' - we don't update the app.

And with the "always_update" key, we always update the app, even if the version doesn't match.  Doesn't that effectively cancel out the key, if we don't respect the version on either install or update?

Not exactly cancels out the "required_platform_version" key. It only removes the restriction of updating the app. That is, the app is always updated without considering the underlying platform version. Maybe we should call this key "always_update_app" to avoid confusion.

The "required_platform_version" key is still effective and controls the OS update to not go beyond the given version.

Okay, thanks for the explanation!

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5000d8c6bb918bf57ea1693aca99524074b78f2f

commit 5000d8c6bb918bf57ea1693aca99524074b78f2f
Author: xiyuan <xiyuan@chromium.org>
Date: Tue Jan 17 22:23:31 2017

kiosk: Add kiosk.always_update manifest key

If the key is specified and its value is true, it allows the app to
be always updated regardless of whether the underlying platform is
compliant or not. If the value is false or the key is not specified,
the required platform version is respected and the app update is
deferred until the underlying platform becomes compliant.

BUG= 673527 

Review-Url: https://codereview.chromium.org/2615633010
Cr-Commit-Position: refs/heads/master@{#444163}

[modify] https://crrev.com/5000d8c6bb918bf57ea1693aca99524074b78f2f/chrome/browser/chromeos/app_mode/kiosk_app_manager.cc
[modify] https://crrev.com/5000d8c6bb918bf57ea1693aca99524074b78f2f/extensions/common/api/_manifest_features.json
[modify] https://crrev.com/5000d8c6bb918bf57ea1693aca99524074b78f2f/extensions/common/manifest_constants.cc
[modify] https://crrev.com/5000d8c6bb918bf57ea1693aca99524074b78f2f/extensions/common/manifest_constants.h
[modify] https://crrev.com/5000d8c6bb918bf57ea1693aca99524074b78f2f/extensions/common/manifest_handlers/kiosk_mode_info.cc
[modify] https://crrev.com/5000d8c6bb918bf57ea1693aca99524074b78f2f/extensions/common/manifest_handlers/kiosk_mode_info.h
[modify] https://crrev.com/5000d8c6bb918bf57ea1693aca99524074b78f2f/extensions/common/manifest_handlers/kiosk_mode_info_unittest.cc
[add] https://crrev.com/5000d8c6bb918bf57ea1693aca99524074b78f2f/extensions/test/data/manifest_tests/kiosk_always_update.json
[add] https://crrev.com/5000d8c6bb918bf57ea1693aca99524074b78f2f/extensions/test/data/manifest_tests/kiosk_always_update_false.json
[add] https://crrev.com/5000d8c6bb918bf57ea1693aca99524074b78f2f/extensions/test/data/manifest_tests/kiosk_always_update_invalid.json

Xiyuan, thanks for putting efforts into this. can you help getting this merged into M56? I know it is close to the stable date.

Let's see what TPM thinks.

This bug requires manual review: We are only 11 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), gkihumba@(cros), bustamante@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

9202.1.0 / 57.0.2987.6 should contain this fix. Harpreet, what can we do to help with the verification?

Mats, yes we are testing using the build you listed. We should be done by Friday EOD barring any unexpected issues.


Xiyuan, observation and question....
If there is a new version of the app available in webstore and "always_update": true key is set in the older version that is currently running on the deivce, the new app gets downloaded but does not start even after rebooting the device. We were able to get the new version of the app started with 'restart ui'.

What is the expected behavior here? Shouldn't the new version of the app launch upon reboot?

The observation is inline with the current implementation. "always_update": true key applies to the app that has it. That is, if the new version of app has the key removed, then the app will not be installed on an incompliant OS. The "always_update" key in old version of app does not affect how the new version is installed.

I can confirm this is what is current implemented and I think it is the expected behavior. Raj/Mats, let me know if you guys think otherwise.

In our case the "always_update": true key is set in both the old and new versions of the app.

Can you clarify "the new app gets downloaded but does not start even after rebooting the device" part? This is not consistent with "We were able to get the new version of the app started with 'restart ui'.".

Did we get the new app running or not?

We could not get the new app running after rebooting the device even though the new version of the app was downloaded as seen in the extensions directory.

But we were able to get the new version of the app to launch after executing "restart ui" command (instead of rebooting the device).

Thanks for the clarification. This looks like a bug. Either reboot or 'restart ui' should run the new app.

Not sure why reboot does not flip the app. It should behave the same as 'restart ui'.

If we still have logs of the reboot case, please attach to the bug. This could save me some time if there is something obvious in it. :)

Sorry, I have rebooted the device before collecting logs. I will try repro this and provide the logs.

​Xiyuan@ - Not able to repro this scenario, where the new version is downloaded to Extensions directory and did not install, hence moving forward with other testing.

Meanwhile, there was a small issue with my manifest file, where I added always_update key value as "always_update":"true" rather "always_update":true (additional "").
Took a while to figure this out and this may be the case where the old app considered as the key being removed in the newer version and could have not installed the newer app. (I am suspecting)​.

Also, the device is not updating and launching the new version app properly. Have to test other scenarios tomorrow.

Thanks for testing. "always_update" uses boolean value. Using a string "true" for its value results in a manifest error. And such extensions/apps would not be installed. That is probably why new version is unpacked but not installed.

This has been tested. What is the plan to merge back to M56? 

Merge here seems obsolete.

Given that no M56 merge is required, this can be closed, right?

Yes, no more work planned on this.