837572 - Cannot access any property by the manifest of app object

Status: RESOLVED FIXED

(Core Graveyard :: DOM: Apps, defect)

Description

[Dale Harvey (:daleharvey)]

Desktop b2g fails for me with the above error, the update tests from https://bugzilla.mozilla.org/show_bug.cgi?id=826058 also fail with the same symptom. 

reverting https://github.com/mozilla/mozilla-central/commit/8d8d2bcf8a31d4f39daf2945c13f3fd401eac814 fixes it for me.

Comment 1

[Dale Harvey (:daleharvey)]

https://tbpl.mozilla.org/php/getParsedLog.php?id=19406158&tree=Try is th elink to the logs for a try run with the update tests that also shows the error:

Error: Permission denied to access property 'name'

Comment 2

[Jason Smith [:jsmith]]

So what exactly would this stop working from at the API level?

Does this basically prevent using app object's manifest.name property?

Comment 3

[[:fabrice] Fabrice Desré]

I know what's happening: we're trying to share an object wrapped in a window on another one.

Comment 4

[Gene Lian (I already quit Mozilla)]

Nominating for tef+ (are we still using tef? this week?) since it looks no good if we cannot access the properties of an app object.

Comment 5

[Gene Lian (I already quit Mozilla)]

Attachment: Patch

A quick patch based on the Fabrice's comment #3. Not yet tested.

Comment 6

[Dale Harvey (:daleharvey)]

jsmith, it stops us from accessing any property on the manifest object, and for triagers it breaks desktop b2g completely.

Comment 7

[Dale Harvey (:daleharvey)]

Tested Gene's patch, works great, cheers

Comment 8

[Gene Lian (I already quit Mozilla)]

(In reply to Dale Harvey (:daleharvey) from comment #7)
> Tested Gene's patch, works great, cheers

Thanks Dale for the quick test!

However, I noticed an issue when trying to solve this bug (please see bug 834999, comment #25). The .uninit() won't be executed when we attempt to use the task manager to kill the app. I'm wondering do we need to move the cahce mechanism to the parent which is capable of listening to the "child-process-shutdown", so that it can know when to clear the cache.

Fabrice, do you have any suggestions?

Comment 9

[[:fabrice] Fabrice Desré]

Comment on attachment 709618 [details] [diff] [review]
Patch

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

Gene, what happens when we close an app from the card view? The process is killed, right? so we don't keep any cache around in the content process I think.

Comment 10

[Gene Lian (I already quit Mozilla)]

(In reply to Fabrice Desré [:fabrice] from comment #9)
> Gene, what happens when we close an app from the card view? The process is
> killed, right? so we don't keep any cache around in the content process I
> think.

Thanks Fabrice for the feedback! :)

The bizarre thing is after I killed an app (ex, Clock app) from card view and relaunched it, it would still use the cached manifest (indexed by manifestURL and innerWindowID). The cached data wasn't cleared as expected... Is there always one Webapps.js for one content process?

(rushing to a meeting and will come back 1.5 hour later)

Comment 11

[[:fabrice] Fabrice Desré]

Are you sure that you're seeing activity from the clock process? and not rather from the homescreen?

Comment 12

[Dale Harvey (:daleharvey)]

The clock app doesnt ever access the manifest object, or even call getSelf so yeh that does seem unlikely

Comment 13

[Gene Lian (I already quit Mozilla)]

Thanks Fabrice and Dale! It should be the Homescreen app and that explains everything. :D Sorry for my misunderstanding. I'll ask for review very soon after more tests!

Comment 14

[Gene Lian (I already quit Mozilla)]

Attachment: Patch, V1.1

Hi Fabrice,

This patch works well. I just corrected the commit message. Thanks for the review again!

Comment 15

[Gene Lian (I already quit Mozilla)]

Comment on attachment 710053 [details] [diff] [review]
Patch, V1.1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 834999
User impact if declined: the content cannot access any properties from the manifest of the app object.
Testing completed: yes
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch: none

Asking for approval‑mozilla‑b2g18+ before tef+ is decided. We definitely need to check this in into b2g18 to fix the regression; otherwise, the app will fail to access the app's properties (like the app's name) through the app object.

Comment 16

[Gene Lian (I already quit Mozilla)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c7fc14709dbd

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c7fc14709dbd

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/1ab85b18f08b
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_0/rev/ea95104e4512

Comment 19

[Gene Lian (I already quit Mozilla)]

Comment on attachment 710053 [details] [diff] [review]
Patch, V1.1

Already landed to b2g18. Remove the approval‑mozilla‑b2g18 request.

Comment 20

[Jason Smith [:jsmith]]

Actually, I'll let automation that Dale has handle the verification.