1221195 - Home screen requests systemXHR permission for no reason

Status: RESOLVED FIXED

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

Description

[Chris Lord [:cwiiis]]

Just noticed this, and I'm pretty sure we don't use it, let's remove it.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] Cwiiis:bug1221195-homescreen-remove-system-xhr > mozilla-b2g:master

Comment 2

[Guillaume Marty [:gmarty]]

Comment on attachment 8682586 [details] [review]
[gaia] Cwiiis:bug1221195-homescreen-remove-system-xhr > mozilla-b2g:master

As discussed offline, it looks good to me.

Comment 3

[Chris Lord [:cwiiis]]

Merged: https://github.com/mozilla-b2g/gaia/commit/6d44ad8956e313e679718df3ac1c393bdd687aec

Comment 4

[Chris Lord [:cwiiis]]

Comment on attachment 8682586 [details] [review]
[gaia] Cwiiis:bug1221195-homescreen-remove-system-xhr > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: Not much - I think it'd be nice to merge this so that people know the minimal set of permissions a home screen should apply for, now that we have replaceable home screens.
[Testing completed]: Tested locally
[Risk to taking this patch] (and alternatives if risky): Hopefully zero, but I don't want to speak too soon
[String changes made]: None

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to revert this in https://github.com/mozilla-b2g/gaia/commit/b6c3b89f2d9ccf8ecc81ba046e5c02f5f7a87067 for breaking a test in gij(22): https://treeherder.mozilla.org/logviewer.html#?job_id=3258999&repo=b2g-inbound

cwiis suggests the test is faulty, but we're reverting this for now to get the tree greened up again.

Comment 6

[Chris Lord [:cwiiis]]

apps/system/test/marionette/import_app_test.js fails because it runs code in the homescreen process that requires this permission - A test shouldn't do this, and looking at the test, there doesn't look like there's any reason it does this on the homescreen app and not on the system app (which would make more sense).

Comment 7

[Chris Lord [:cwiiis]]

Re it making more sense, I'm not sure of that - the activity it's testing is handled by system, so perhaps that's why the code isn't run in system?

Kevin, re comment #6, is there any reason in that test the script is run from the home screen and not from system? Home screen shouldn't need systemXHR, but this test forces its requirement.

Comment 8

[GitHub Autolander Bot]

Attachment: [gaia] Cwiiis:bug1221195-homescreen-remove-system-xhr > mozilla-b2g:master

Comment 9

[Chris Lord [:cwiiis]]

Comment on attachment 8683565 [details] [review]
[gaia] Cwiiis:bug1221195-homescreen-remove-system-xhr > mozilla-b2g:master

Rather than needinfo, here's the PR with the amended test.

Comment 10

[Chris Lord [:cwiiis]]

Comment on attachment 8683565 [details] [review]
[gaia] Cwiiis:bug1221195-homescreen-remove-system-xhr > mozilla-b2g:master

Carrying r=gmarty

Comment 11

[Kevin Grandon :kgrandon]

Comment on attachment 8683565 [details] [review]
[gaia] Cwiiis:bug1221195-homescreen-remove-system-xhr > mozilla-b2g:master

Seems fine to me, thanks!

Comment 12

[Chris Lord [:cwiiis]]

Thanks, merged: https://github.com/mozilla-b2g/gaia/commit/ccc064c3d32bc9bb6787e768a761055f4fb69769

Comment 13

[Chris Lord [:cwiiis]]

Comment on attachment 8683565 [details] [review]
[gaia] Cwiiis:bug1221195-homescreen-remove-system-xhr > mozilla-b2g:master

Let's try this again :)

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: Good to show the minimal set of permissions a home screen needs for users who are interested in developing a new home screen.
[Testing completed]: Tested locally, seems fine.
[Risk to taking this patch] (and alternatives if risky): Low
[String changes made]: None

Comment 14

[Mahendra Potharaju [:mahe]]

Comment on attachment 8683565 [details] [review]
[gaia] Cwiiis:bug1221195-homescreen-remove-system-xhr > mozilla-b2g:master

Approved for 2.5 uplift. 

Thanks

Comment 15

[Carsten Book [:Tomcat]]

landed on 2.5 https://github.com/mozilla-b2g/gaia/commit/4c0f29fb84ad0c45b3a1d2c462c08f808d692ec7