820863 - Disable usage of the entry points property by third party apps during app install

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System, defect, P2)

Description

[Jason Smith [:jsmith]]

Per talking with a few people, there has been a concern raised about the "entry points" app manifest property being:

- A highly buggy implementation
- An edge case by developers

In planning my test plan, Julien confirmed that he thought the entry points pieces could be buggy. In immediately testing it quickly, I've already seen some fundamental problems such as:

- Uninstalling a hosted app with entry points does not remove the specific app icon entry point until a restart
- Installing a packaged app with entry points fails - only one app icon for the general app is added, not the entry point pieces

Given that we are short on time, I'm going to put out a proposal that we disable the ability for third-party apps to use the entry points app manifest property - only our internal Gaia apps can use it for v1. I think this is a safe decision from balancing likelihood of usage and effort to get this thing working.

Comment 1

[Ben Francis [:benfrancis]]

Vivien, can you provide input on whether you think we should try to fix this properly or do the work to disable for third party apps?

Comment 2

[[:fabrice] Fabrice Desré]

That's clearly a gaia bug, and I don't want to forbid that at the platform level because of that. Note that for the Marketplace, filtering could be done there when validating the manifest.

Comment 3

[Julien Wajsberg [:julienw]]

I think the amount of work to support this is not so big, and it would help a lot to know the precise use cases that are currently failing. 

I thought that mainly the cancel/restart functionality were broken but your tests suggest otherwise.

Comment 4

[Jason Smith [:jsmith]]

(In reply to Ben Francis [:benfrancis] from comment #1)
> Vivien, can you provide input on whether you think we should try to fix this
> properly or do the work to disable for third party apps?

Or also address the third piece - which is do nothing and don't block ship. Basically I'm looking to understand is should we:

1. Find and fix the bugs with entry points and treat them as blockers
2. Disable the functionality in "gaia" for parsing the entry points
3. Do nothing and don't block

Note - I'm not a hard stance on the disabling piece - if we feel confident we can fight the battle here to get this working, then let's try to fix it. But it might be worth to analyze the tradeoffs here.

Comment 5

[Jason Smith [:jsmith]]

Also adding needs info on akeybl to give the rel management tradeoff perspective here.

Comment 6

[Francisco Jordano [:arcturus] [:francisco]]

Hi folks,

I have my concerns regarding use cases like:

- Uninstall: we cannot maintain a registry of entry points, so or either you lose all the entry points or no one.
- Retry: Should't be a problem, you get everything or nothing.
- Cancelling: Same, shouldn't be a problem.

Cheers,
F.

Comment 7

[Jason Smith [:jsmith]]

Per bug 821200 - needs a product decision.

Comment 8

[Alex Keybl [:akeybl]]

(In reply to Jason Smith [:jsmith] from comment #5)
> Also adding needs info on akeybl to give the rel management tradeoff
> perspective here.

Multiple entry points for 3rd parties is not supported on iOS, and is hardly used in  Android apps. Entry points for 3rd party apps should be out of scope for v1.0 at this point.

Comment 9

[Alex Keybl [:akeybl]]

I strongly suggest +ing this bug and disabling entry points into third party apps.

Comment 10

[Julien Wajsberg [:julienw]]

So we want to disable entry points only for 3rd party apps, but keep them for apps that are preinstalled, did I understand this correctly ?

So the plan is :
- forbid entry points for installation
-> what should we do if the user tries to install an app with entry-points ?
-> proposition: like an "install failed" error (a small notice + a log for the developer)

- can the number of entry-points change after an update ?
-> If yes, then we must check this during update as well.
-> If this happens, what should we do ? This could be a problem if the user can't update an app (although the app author would notice this before and therefore would fix this before any user tries to update).

- we can remove all entry point support for adding icons to the homescreen
- AFAIK we can't uninstall preinstalled apps so we can remove all entry point support for the uninstall feature.
- we must keep the support for launching apps.

Comment 11

[Peter Dolanjski [:pdol]]

I agree with Alex here - let's move out of scope for v1.0. (Thanks for +ing David)  It won't be a common use case out of the gate.

Julien, yes, disable for 3rd party, keep for preinstalled.

I'll let jcarpenter comment on the user experience in each of the scenarios.

Comment 12

[Faramarz Rashed]

:julienw, let's fix this as soon as possible please.  it's been sitting untouched for 4 days.

Comment 13

[Alex Keybl [:akeybl]]

(In reply to Julien Wajsberg [:julienw] from comment #10)
> So we want to disable entry points only for 3rd party apps, but keep them
> for apps that are preinstalled, did I understand this correctly ?
> 
> So the plan is :
> - forbid entry points for installation
> -> what should we do if the user tries to install an app with entry-points ?
> -> proposition: like an "install failed" error (a small notice + a log for
> the developer)
> 
> - can the number of entry-points change after an update ?
> -> If yes, then we must check this during update as well.
> -> If this happens, what should we do ? This could be a problem if the user
> can't update an app (although the app author would notice this before and
> therefore would fix this before any user tries to update).
> 
> - we can remove all entry point support for adding icons to the homescreen
> - AFAIK we can't uninstall preinstalled apps so we can remove all entry
> point support for the uninstall feature.
> - we must keep the support for launching apps.

Discussed during triage. We should just throw an existing generic install error when the user tries to install/update an app with multiple entry points. Given that, removing uninstall support should not be necessary. Let's move forward here.

Comment 14

[Josh Carpenter [:jcarpenter]]

Agreed w/ Peter.

Comment 15

[Julien Wajsberg [:julienw]]

faramarz> should have a patch in a few hours. I was waiting for the UX part that I just had.

Comment 16

[Julien Wajsberg [:julienw]]

I was trying to find how to do disable this and I believe this is not doable in Gaia for the following case.


Before installing a packaged app, we don't have access to the full manifest (where the entry_points are defined) because it's inside the ZIP file. So in Gaia we only get access so the full manifest when the app is downloaded and installed because the app is installed as soon as the download is finished without any gaia interaction.


All other cases should be doable, however because of this last case I believe this should be prevented in the platform for now, and we must definitely open a follow-up for post-v1.

Comment 17

[[:fabrice] Fabrice Desré]

The platform also needs to download the package to see the entry points... We can only abort slightly earlier.

Comment 18

[Julien Wajsberg [:julienw]]

Fabrice> in Gaia we can't abort before install, in the platform you can do this (between download and install).

Comment 19

[[:fabrice] Fabrice Desré]

Yes, but from a user point of view, he would have downloaded a potentially large package for nothing. That's not good.

Comment 20

[Julien Wajsberg [:julienw]]

That's exactly the same if the manifest is invalid for some other reason...

Another possibility is to ignore the entry_points property if it's present (with log for the dev). I think this requires work in Gecko (it would sanitize the manifest so that the Gaia code would see a normal manifest without entry_points), but I'm not sure the amount of work is less than just properly supporting this in Gaia.

Comment 21

[Julien Wajsberg [:julienw]]

Thinking about removing the support of entry_points for update, I am also concerned with the update of preloaded apps: their entry_points must not change if we do this, because we won't read them.

The more I think of this, the more I think we should just support it completely. I think we would have done it already...

Fabrice, what do you think ? Should we ask the drivers to reconsider this decision ?

Comment 22

[Julien Wajsberg [:julienw]]

Or maybe just keep it like it is now and implement properly post-v1.

The only case I consider important is updating pre-installed apps with entry-points. Could you check this case Jason ?

Comment 23

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v1

There is not time to fix/investigate all the possible bugs that can happens with third party apps that use entry points. Let's allow them only for certified apps only for now and we can fix that post-v1.

Comment 24

[Julien Wajsberg [:julienw]]

Comment on attachment 694468 [details] [diff] [review]
Patch v1

Review of attachment 694468 [details] [diff] [review]:
-----------------------------------------------------------------

(stealing review as we talked about this with Vivien)

Seems correct to me. r=julienw

next time with unit tests ? ;)

Comment 25

[Julien Wajsberg [:julienw]]

In this patch, we ignore entry_points for non-certified app, but only in gaia. This means that Gecko still knows about entry points but it doesn't seem to be a problem.

Comment 26

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://github.com/mozilla-b2g/gaia/commit/c6d071096958026cff69d3798fb9b219d2c51f5f

Comment 27

[Jason Smith [:jsmith]]

Verified with the two test apps on apps.cloudfoundry.com for hosted and packaged respectively.

Feel free to file a bug for bookkeeping btw that is something along the lines of adding back support for entry points post v1.