1200851 - [Home Screen] Unable to add bookmarks to New homescreen

Status: VERIFIED FIXED

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

Description

[[:VladimirBelonenko]]

Attachment: logcat_20150901_1435.txt

Description:
When the user attempts to add bookmark to the New home screen, it does not appear to be added to Home screen.

Repro Steps:
1) Update a Aries to 20150901190824
2) Launch browser.
3) Navigate to a web site.
4) Select "..." button.
5) Select Add to home screen.
6) Navigate to home screen

Actual:
The link is not added to the New home screen.

Expected:
Link will be displayed on the New home screen successfully.

Environmental Variables:
Device: Aries 2.5 Kk
Build ID: 20150901190824
Gaia: c2582f4be03cd12124b96a263c8d14c774f0ffe4
Gecko: e47423c019643792e6de894cfcee598dead4d3ba
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 43.0a1 (2.5)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Repro frequency: 3/3
See attached:Video in logcat.

Comment 1

[[:VladimirBelonenko]]

This issue also occurs on Flame 2.5
Result: The link is not added to NEW home screen.

Environmental Variables:
Device: Flame 2.5 KK Full Flash (319mb)
Build ID: 20150831133012
Gaia: c80e8ff25425b007181fd6e3de0500a0358fab37
Gecko: cafb1c90f794a73100a8f0afb9fe3301df0f2bde
Gonk: c4779d6da0f85894b1f78f0351b43f2949e8decd
Version: 43.0a1 (2.5)
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

--------------------------------------

This issue does not occur on Flame 2.2
Result: New home screen does not support Flame 2.2

Environmental Variables:
Device: Flame 2.2 kk Full Flash (319 mb)
Build ID: 20150901003002
Gaia: 335cd8e79c20f8d8e93a6efc9b97cc0ec17b5a46
Gecko: 504665629f49
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 37.0 (2.2)
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Comment 2

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I was able to add a link the the homescreen.
I think the difference might be that I'm using an engineering build.  This also might be based on getting to a certain state.  What other things have you done prior to testing this?

The logcat doesn't show anything from yahoo like the video does.
It's not clear from the logcat when you tried to do certain tasks.  Can you please narrow the logcat to the events that you did?

Comment 3

[Chris Lord [:cwiiis]]

I'm not going to close this just yet, but this works for me *and* is covered by an integration test (that also works). If you're installing your own builds, it's quite easy to get into a state where datastore access won't be correctly granted.

It's worth trying to update Gecko (this requires bug 1181329), and also updating the buildID pref, but I'm pretty certain this works. If anyone can give solid STR, perhaps we can narrow down what circumstances cause this to happen and work around them/warn the user somehow?

Comment 4

[[:VladimirBelonenko]]

Attachment: logcat_20150902_1128.txt

I just updated a log with my steps I performed.
You need to change settings Homescreen to New Homescreen.

Comment 5

[Chris Lord [:cwiiis]]

Going on your log, this is happening because the homescreen isn't getting datastore access:

09-02 11:28:27.237  1780  1780 E New Home Screen: Content JS ERROR: places inaccessible 
09-02 11:28:27.237  1780  1780 E New Home Screen:     at Datastore.prototype.init/</req.onsuccess/< (app://homescreen.gaiamobile.org/gaia_build_defer_index.js:778:72)
09-02 11:28:27.247   319   319 E GeckoConsole: [JavaScript Error: "ConstraintError" {file: "jar:file:///system/b2g/omni.ja!/components/DataStoreImpl.js" line: 233}]
09-02 11:28:29.297  1780  1780 E New Home Screen: Content JS ERROR: bookmarks_store inaccessible 
09-02 11:28:29.297  1780  1780 E New Home Screen:     at Datastore.prototype.init/</req.onsuccess/< (app://homescreen.gaiamobile.org/gaia_build_defer_index.js:778:72)

Not sure what the ConstraintError part is...

The problem is, we don't know why this is happening. If you're running with recent Gecko, this shouldn't be a problem. What version of Gecko are you running? Can you check your 'dom.mozApps.homescreenURL' pref and make sure it corresponds to the 'homescreen.manifestURL' setting? (you can check both of these using Runtime -> Device Preferences and Runtime -> Device Settings in the Web IDE).

Comment 6

[Chris Lord [:cwiiis]]

So Guillaume has narrowed this down to this happening the first time after a clean install, but is fixed after rebooting. I wonder if the pref read in Gecko ends up getting cached, and/or there's a race-condition there?

I'll have a look and see if I can debug this in Gecko.

Comment 7

[Chris Lord [:cwiiis]]

This is interesting - Instrumenting Gecko, I can see that DataStoreService::CheckPermission is succeeding after switching to the new homescreen, but the returned list doesn't include the datastores that were asked for all the same...

We have to go deeper.

Comment 8

[Chris Lord [:cwiiis]]

I'm seeing

E/GeckoConsole(10890): [JavaScript Error: "DataCloneError: The object could not be cloned." {file: "jar:file:///system/b2g/omni.ja!/components/DataStoreImpl.js" line: 233}]

in my log, which is the same line highlighted in comment #5, but a different error... I'm guessing this isn't benign...

n?Andrea in case they have any insight.

Comment 9

[Chris Lord [:cwiiis]]

This is stumping me, there's too much code to dig through for me to debug this in any reasonable time vs. someone that actually knows the code - Andrea, if you could have a look, that'd be most appreciated.

STR:

- Flash Gecko/Gaia (anything reasonably recent - like in the last couple of weeks)
- Go to settings
- Pick 'Homescreens' (Homescreen*s*, not Homescreen - unless Bug 1180666 has landed)
- Change to 'New Home Screen'

Expected:

No datastore-related error messages in the console

Actual:

navigator.getDataStores returns an empty array and a DataCloneError is printed on the console:
E/GeckoConsole(14824): [JavaScript Error: "DataCloneError: The object could not be cloned." {file: "jar:file:///system/b2g/omni.ja!/components/DataStoreImpl.js" line: 233}]

Note: Restarting the device reportedly fixes the issue from this point, and changing the app permissions to certified also fixes the issue. This issue only happens going from a clean slate, before restarting. This issue does not seem to replicate on integration testing, which sees no problem (maybe an e10s-related error?)

Comment 10

[Andrea Marchesini [:baku]]

Is it a regression? If yes, can you tell me when it stopped to work?

Comment 11

[Chris Lord [:cwiiis]]

(In reply to Andrea Marchesini (:baku) from comment #10)
> Is it a regression? If yes, can you tell me when it stopped to work?

I don't think this is a regression, but I'm uncertain. It's hard to know because it disappears after a reboot, but this always manifests after a clean gaia install, after switching to the new homescreen, and it remains that way until rebooting. Do you have any cycles to look into this, or can you help me debug this?

Comment 12

[Andrea Marchesini [:baku]]

I found a way to reproduce the issue.
What I see is that gaia code tries to store an object like this:

{ blob: <Blob>,
  originalUrl: <URL>,
  timestamp: <number> }

the problem is that URLs are not supported by our StructuredCloneAlgorithm and that makes the operation to fail.
I see 2 options:

1. we can support URL in the StructuredCloneAlgorithm forcing them to string (this is probably what gaia teams want)
2. we change the gaia code so that we do: originalUrl + "", or any other way to convert a URL into a string.

Comment 13

[Chris Lord [:cwiiis]]

We've been discussing this on IRC, and unfortunately the problem mentioned in comment #12 is a separate bug (filed bug 1203980). Fixing this, the DataCloneError disappears, but the stores remain inaccessible. There doesn't seem to be any relevant error output in logcat.

Comment 14

[Andrea Marchesini [:baku]]

I'll debug this issue next week.

Comment 16

[Andrea Marchesini [:baku]]

I know why we have this issue: we have a special permission check for the homescreen so that this particular app can have access to DataStores also if it's not certificated. The way this is done is using a preference: dom.mozApps.homescreenURL.
that has to contain the manifestURL of the current homescreen app.

When we change the homescreen, this pref is maybe updated, but the DataStoreService doesn't know anything about it.
The result is that: the new homescreen cannot have access to the DataStore and the old homescreen can.

I'm going to fix this issue soon.

Comment 17

[Andrea Marchesini [:baku]]

Attachment: aa.patch

Comment 18

[[:fabrice] Fabrice Desré]

Comment on attachment 8661683 [details] [diff] [review]
aa.patch

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

r- because I'd really prefer using the localId in the AppsService contract.

::: dom/apps/Webapps.jsm
@@ +4649,5 @@
>      return OS.Path.dirname(this.appsFile);
>    },
>  
> +  updateDataStoreEntriesFromAppId: function(aAppId) {
> +    this.updateDataStoreForApp(aAppId);

Would will have to get the appId from the localId there.

::: dom/interfaces/apps/nsIAppsService.idl
@@ +93,5 @@
> +
> +  /**
> +   * Reads the manifest file for this app and update the DataStore map
> +   */
> +  void updateDataStoreEntriesFromAppId(in DOMString appId);

I'd like that to use the localId instead. the appId string should not have been exposed in the first place.

Comment 19

[Andrea Marchesini [:baku]]

Attachment: aa.patch

Comment 20

[Andrea Marchesini [:baku]]

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

Comment 21

[Pulsebot]

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

Comment 22

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

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

Comment 23

[Shally Li（leave from Mozilla）]

This bug has been verified as "pass" on the latest build of Flame KK 2.5 and Aires KK 2.5 by the STR in comment 0.

Actual results: Users can add bookmarks to New homescreen.
See attachment: verified_Aries_v2.5.3gp
Reproduce rate: 0/10


Device: Flame KK 2.5 (Pass)
Build ID               20150924150202
Gaia Revision          4bb17b24620818cbda0ba0c0d69e0ce3f914e1b7
Gaia Date              2015-09-23 16:06:39
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/001942e4617b2324bfa6cdfb1155581cbc3f0cc4
Gecko Version          44.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150924.183612
Firmware Date          Thu Sep 24 18:36:29 EDT 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Device: Aries KK 2.5 (Pass)
Build ID               20150925003138
Gaia Revision          4bb17b24620818cbda0ba0c0d69e0ce3f914e1b7
Gaia Date              2015-09-23 16:06:39
Gecko Revision         https://hg.mozilla.org/integration/mozilla-inbound/rev/eee4266046984718e4daa99d94ce820f3fd86d32
Gecko Version          44.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150925.001452
Firmware Date          Fri Sep 25 00:15:01 UTC 2015
Bootloader             s1

Comment 24

[Shally Li（leave from Mozilla）]

Attachment: verified_Aires_v2.5.3gp