1042807 - Vertical home screen should use homescreen-webapps-manage permission

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Homescreen, defect)

Description

[Ted Clancy [:tedders1]]

Under the new permission model, vertical homescreen can't access the collection icons because they live in a different app.

We need to put the icons somewhere that vertical homescreen can access them.


+++ This bug was initially created as a clone of Bug #1000305 +++

Comment 1

[Ted Clancy [:tedders1]]

Attachment: bug-1042807.patch

This patch contains changes that make the vertical homescreen work with the new icon API and the new permission model.

With this patch, the collections app copies the collection icons into a datastore as a blob, and the vertical homescreen extracts them from the datastore.

However, I've noticed this makes the vertical homescreen app start up slower, so this solution might be unsatisfactory. I want to investigate a different solution if I get a change.

Comment 2

[Ted Clancy [:tedders1]]

*if I get a chance

Comment 3

[Kevin Grandon :kgrandon]

Comment on attachment 8462748 [details] [diff] [review]
bug-1042807.patch

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

Hey Ted,

This looks good, but my main question is why do we need a new property on the collection object? It looks like we already have a decoratedIconBlob property - can we reuse that?

Also do you know where the slowdown on startup is coming from? Is it due to the get getIcon() method? Startup concerns are super important for 2.0 now, so if this regresses startup time we are going to have a bad time =/

Comment 4

[Kevin Grandon :kgrandon]

Since bug 1092726 has been marked as a WONTFIX for now, I think we can also keep this as a wontfix?

Feel free to re-open if necessary.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

3rd party homescreens still can't access content from other apps since they aren't certified.

But once bug 1000305 is fixed, 3rd party homescreens can access icons from other apps. Is that enough for what we need here?

Comment 6

[Kevin Grandon :kgrandon]

If we want to convert the vertical home screen to use homescreen-webapps-manage, then we would still want the patch here. Not really something that I think we need to do right now, but I could be convinced. The main benefit I can think of is third party developers looking at our code as an example.

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I definitely think that we should limit our homescreen to privileged APIs (and capabilities) so that we can be sure that 3rd party homescreens can be as good as ours.

Comment 8

[Kevin Grandon :kgrandon]

Sounds good to me then. Let's keep this around to track the porting to the homescreen-webapps-manage permission.

Comment 9

[[:fabrice] Fabrice Desré]

Ted, can you rebase this patch?

Comment 10

[Ted Clancy [:tedders1]]

I'm sorry for not doing this yet. I should be able to do it next week.

Comment 11

[Ted Clancy [:tedders1]]

Attachment: bug-1042807-rebased.patch

Rebased.

Comment 12

[Ted Clancy [:tedders1]]

Fabrice, do you want me to land this?

Comment 13

[[:fabrice] Fabrice Desré]

I think that would not hurt, but I defer the review to kgrandon.

Comment 14

[Kevin Grandon :kgrandon]

Comment on attachment 8664754 [details] [diff] [review]
bug-1042807-rebased.patch

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

I am fine with doing something like this, but shouldn't we remove the webapps-manage permission if so? Please let me know why or why not and flag me again, thanks.

Comment 15

[GitHub Autolander Bot]

Attachment: [gaia] tedders1:bug-1042807-fix > mozilla-b2g:master

Comment 16

[Ted Clancy [:tedders1]]

(In reply to Kevin Grandon :kgrandon from comment #14)
> Comment on attachment 8664754 [details] [diff] [review]
> shouldn't we remove the webapps-manage permission if so?

Quite right.

Comment 17

[Kevin Grandon :kgrandon]

Comment on attachment 8665046 [details] [review]
[gaia] tedders1:bug-1042807-fix > mozilla-b2g:master

Ted - 

I gave this a shot on the device and all app icons were defaulted to the rocketship - I'm not entirely sure why. Was this working for you after a `make reset-gaia`? I tried using the latest gecko build, did all necessary gecko patches land?

Comment 18

[Ted Clancy [:tedders1]]

Oops. I did check on a device, but I guess I didn't flash the phone properly. (And it passed the automated tests.)

New version pushed.

Comment 19

[Kevin Grandon :kgrandon]

Comment on attachment 8665046 [details] [review]
[gaia] tedders1:bug-1042807-fix > mozilla-b2g:master

I really want to land this patch, but for some reason every other time I flash this an icon or two is missing from the home page. I'm running the latest gecko and am testing with a flame. 

Ted - it feels like there's a race condition somewhere, and I'm not sure if it's in the home screen code, or in gecko. Any ideas?

Comment 20

[Ted Clancy [:tedders1]]

> but for some reason every other time I flash this an icon or two is missing from the home page.

How weird. I just flashed my phone 5 times, and not once did I see any icons missing.

I'm also using a flame, but I'm not sure I have the latest gecko.

Can you tell me what icon is missing? Is it a Marketplace app, or a pre-installed webapp? Maybe knowing that might help me reproduce the problem.

Comment 21

[Kevin Grandon :kgrandon]

It varied, I will try to repro again soon.

Comment 22

[Kevin Grandon :kgrandon]

Attachment: Screenshot - missing icons after flash

(In reply to Ted Clancy [:tedders1] from comment #20)
> Can you tell me what icon is missing? Is it a Marketplace app, or a
> pre-installed webapp? Maybe knowing that might help me reproduce the problem.

It appears that it can happen to any installed app after a reset-gaia. It's totally random and sometimes it's 0 icons missing, or sometimes 4. Here's a screenshot with 4 missing icons. (Typically they will return after a restart, but my hunch is that's because we're rendering some cached icons at that point).

Comment 23

[Kevin Grandon :kgrandon]

This also happens with the new homescreen so it seems there might be a problem with the api/platform: https://bugzilla.mozilla.org/show_bug.cgi?id=1211266

Comment 24

[Chris Lord [:cwiiis]]

I've also never experienced this, but I've only been testing on an aries device. This blocks new homescreen, which blocks pin-the-web. Is there anyone actively looking at it?

Comment 25

[Ted Clancy [:tedders1]]

Kevin, if that's a separate issue from this ticket, can this ticket land? Or is that blocking this ticket?

Comment 26

[Kevin Grandon :kgrandon]

(In reply to Ted Clancy [:tedders1] from comment #25)
> Kevin, if that's a separate issue from this ticket, can this ticket land? Or
> is that blocking this ticket?

It's a separate issue technically. I filed bug 1211266 to better capture the issue, so maybe we should track it there, or re-purpose that one to specifically fix the platform issue.

Comment 27

[Kevin Grandon :kgrandon]

Since this is no longer the default, the platform issue is being fixed in bug 1211266, and qa wants a privileged home screen - may as well land to close this out so we can ship it as a privileged one if we want. Thanks Ted!

In master: https://github.com/mozilla-b2g/gaia/commit/2e89362de40a6c9c36525d36317fa1ae8e67e143