Closed Bug 1166044 Opened 9 years ago Closed 9 years ago

P2PSharing : black when launched

Categories

(Firefox OS Graveyard :: Gaia::P2P Sharing, defect)

Product:
Firefox OS Graveyard
Component:
Gaia::P2P Sharing
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(blocking-b2g:2.5+, b2g-master fixed)

Status:
VERIFIED FIXED
Milestone:
2.2 S13 (29may)
Project Flags:
blocking-b2g 2.5+
Tracking Flags:
Tracking Status
b2g-master --- fixed

People

(Reporter: nhirata, Assigned: drs)

References

Details

(Keywords: qablocker, Whiteboard: [spark])

Attachments

(3 files)

2015-05-18-16-15-35.png
17.50 KB, image/png
Details
logcat.txt
6.06 MB, text/plain
Details
Use new r.js optimizer to resolve dependencies correctly without requiring `toSource()` to work.
39 bytes, text/x-github-pull-request
justindarc
: review+
Details | Review 
Gaia-Rev        7a811cdd9ebcd7924d3745421c564d4c69b1bb87
Gecko-Rev       https://hg.mozilla.org/projects/cypress/rev/7147927d49b0
Build-ID        20150514163125
Version         40.0a1
Device-Name     aries
FW-Release      4.4.2
FW-Incremental  eng.worker.20150514.153229
FW-Date         Thu May 14 15:32:37 UTC 2015
Bootloader      s1

hrm.  today's build is still using cypress...
Attached file logcat.txt — 
logcat is showing : 

E/Sharing ( 1481): [JavaScript Error: "Error: Timeout for modules: app/js/index,app/js/controllers/main_controller,app/js/controllers/progress_dialog_controller,app/js/controllers/proximity_apps_controller,app/js/controllers/share_controller,app/js/views/app_view,app/js/views/device_name_view,app/js/views/progress_dialog_view,app/js/views/share_summary_view,app/js/views/list_view,app/js/views/share_settings_view,gaia-button/gaia-button,gaia-text-input/gaia-text-input,gaia-dialog/gaia-dialog-prompt,gaia-progress/gaia-progress,gaia-list/gaia-list,gaia-checkbox/gaia-checkbox,gaia-sub-header/gaia-sub-header,gaia-loading/gaia-loading,gaia-switch/gaia-switch" {file: "app://sharing.gaiamobile.org/js/initapp.js" line: 1114}]
Assignee: nobody → drs
[Blocking Requested - why for this release]:
blocking-b2g: --- → spark?
Whiteboard: [spark]
Depends on: 1168645
blocking-b2g: spark? → spark+
The issue with user builds and this wanting to be a "certified" app. That combination strips Function.prototype.toString values from being useful, so the gaia-components that use the define(function(require) {}) syntax do not get their dependencies parsed correctly.

If the app could avoid being certified that would be one fix. I don't think sources are stripped for non-certified apps.

I came up with this change locally to normalize the define() calls in dist/app/components to have their dependency arrays:
diff --git a/package.json b/package.json
index 0ae6466..2f76d91 100644
--- a/package.json
+++ b/package.json
@@ -15,6 +15,8 @@
   "dependencies": {
     "fxos-build": "0.7.x",
     "gulp": "3.8.x",
-    "adapt-pkg-main": "^1.0.0"
+    "adapt-pkg-main": "^1.0.0",
+    "glob": "5.0.x",
+    "requirejs": "2.1.x"
   }
 }

And introduced a normalize-components.js script at the top level of the repo that can be run via `node normalize-components.js`. Its contents:

---

'use strict';
/*global require, __dirname, console */

var requirejs = require('requirejs'),
    glob = require('glob'),
    fs = require('fs'),
    path = require('path'),
    targetDir = path.join(__dirname, 'dist', 'app', 'components');


//Do it twice, just to make sure it all holds together on multiple passes.
requirejs.tools.useLib(function(require) {
  require(['transform'], function (transform) {
    // options is optional
    glob('**/*.js', {
      cwd: targetDir,
      nodir: true
    }, function (err, files) {
      if (err) {
        return console.error(err);
      }

      files.forEach(function(filePath) {
        var fullPath = path.join(targetDir, filePath);
        var contents = fs.readFileSync(fullPath, 'utf8');
        var transformed = transform
                          .toTransport('', null, fullPath, contents);

        if (transformed !== contents) {
          fs.writeFileSync(fullPath, transformed, 'utf8');
        }
      });
    });
  });
});

---

The unfortunate parts was that transform.toTransport depends on esprima 2.1 for AST parsing, and while it understands some ES6 stuff it apparently has a problem with template strings, which are used in a few components, so still get load failures. gaia-button is an example, line 48 is where the AST parsing fails, so those files are not normalized and the error persists.

Note that this is not a problem using the requirejs optimizer via xpcshell, as the gaia build does, where it delegates to Reflect.parse() instead of esprima, and it understands the template strings used.

So this is at a hard point between too much bleeding edge technology. Possible options on fixes:

1) If the app can move to not be a "certified" app, that might fix it without needing these define() normalizations.

2) The latest release of the r.js optimizer uses esprima 2.1, but esprima 2.2 is out, and I will try tomorrow to see if an upgrade to that esprima avoids the template string problem. Note that if the gaia-components decide to use fancier JS that is not supported by esprima, there will be a risk of this breaking in the future.

3) gaia components move to array syntax defines to avoid needing Function.toString(). Not very pretty for those authors.

4) Somehow drive r.js via xpcshell instead of node. I do not have any immediate idea on how to do that, or if it is palatable for this type of app, since it means probably using something like mozilla-download to download a version of firefox with xpcshell.

I will report back on option 2 tomorrow.
I went ahead and tried esprima 2.2, and that seems to work at least for initial load. Now only these files are skipped in the toTransport step:

toTransport skipping /Users/jr/git/temp/sharing/dist/app/components/fxos-achievements-service/achievements-service.js: Error: Line 5: Unexpected token
toTransport skipping /Users/jr/git/temp/sharing/dist/app/components/fxos-mvc/mvc.js: Error: Line 4: Unexpected token
toTransport skipping /Users/jr/git/temp/sharing/dist/app/components/fxos-settings-utils/settings-utils.js: Error: Line 5: Unexpected token

First file is still in babel format, so these might be files that are not actually loaded in the app for distribution.

I updated r.js master to include esprima 2.2. It was already a planned upgrade for the 2.1.18 r.js release, but I still have some other things to fix before doing a formal release for that. In the meantime, you could download the master snapshot here:

https://raw.githubusercontent.com/jrburke/r.js/master/dist/r.js

I did that, saved it as r.js alongside normalize-components.js then changed that file to require('./r') instead of 'requirejs' and got a dist/app version that seemed to load. I get a "TypeError: _this.els is undefined" error, but the UI seems to come up.
Blocks: 1169067
Amazing investigation. Thanks, James. This fixes the issue for me as well.
Attachment #8611523 - Flags: review?(jdarcangelo)
Comment on attachment 8611523 [details] [review]
Use new r.js optimizer to resolve dependencies correctly without requiring `toSource()` to work.

LGTM. Do we need to do this for other apps using fxos-build?
Attachment #8611523 - Flags: review?(jdarcangelo) → review+
(In reply to Justin D'Arcangelo [:justindarc] from comment #9)
> LGTM. Do we need to do this for other apps using fxos-build?

Theoretically, yes, but in practice, none of them are as complicated and dependency-heavy as the Sharing app. Let's evaluate on a case-by-case basis, since this solution is so far from ideal.

https://github.com/fxos/sharing/commit/5a678a89b7a78c8bc986b85282bcd6c3d19b6bb0
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
No longer all black on basic themed.

Build ID               20150603204744
Gaia Revision          9e10483c5808f94f4a0a9f6afe30aae2c5b42b4c
Gaia Date              2015-06-03 19:22:33
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/0920f2325a6d
Gecko Version          41.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150603.203317
Firmware Date          Wed Jun  3 20:33:25 UTC 2015
Bootloader             s1
Status: RESOLVED → VERIFIED
blocking-b2g: spark+ → 2.5+
status-b2g-v2.5: --- → fixed
status-b2g-master: --- → fixed
Target Milestone: --- → 2.2 S13 (29may)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: