Closed Bug 1028037 Opened 6 years ago Closed 6 years ago

NFC testcase failure with error message "Accessing TypedArray data over Xrays is slow ..."

Categories

(Firefox OS Graveyard :: NFC, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
2.0 S5 (4july)

People

(Reporter: dimi, Assigned: dimi)

References

Details

(Whiteboard: [perf-reviewed])

Attachments

(1 file, 3 obsolete files)

When running nfc testcase , it shows following error message

GeckoConsole: [JavaScript Error: "Accessing TypedArray data over Xrays is slow, and forbidden in order to encourage performant code. To copy TypedArrays across origin boundaries, consider using Components.utils.cloneInto()."
Hi Jeff,
  The NFC testcase worked fine before, but show this error message now.
Is it related to the TypedArray changed recently ? Do you have any suggestion ? Thanks!
Flags: needinfo?(jwalden+bmo)
This is due to bug 987163, not any of the things I've landed recently.

http://hg.mozilla.org/mozilla-central/rev/4a50fbe99969

You want to do what the message says, use Cu.cloneInto() to create a typed array in whatever window it is you're passing a chrome-created typed array into.
Flags: needinfo?(jwalden+bmo)
(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #2)
> You want to do what the message says, use Cu.cloneInto() to create a typed
> array in whatever window it is you're passing a chrome-created typed array
> into.

I think it's the other way around - accessing a content-created typed array from chrome.

Dimi, can you describe the use-case?
(In reply to Bobby Holley (:bholley) from comment #3)
> (In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #2)
> > You want to do what the message says, use Cu.cloneInto() to create a typed
> > array in whatever window it is you're passing a chrome-created typed array
> > into.
> 
> I think it's the other way around - accessing a content-created typed array
> from chrome.
> 
> Dimi, can you describe the use-case?

There is a webidl file named MozNDEFRecord.webidl containing the NFC data using Uint8Array.
The GAIA system app or testcase itself will creat the MozNDEFRecord to compare or process the data.

The error occurs when trying to read content inside MozNDEFRecord like type, id , and payload.
(In reply to Dimi Lee[:dimi][:dlee] from comment #4)
> The GAIA system app or testcase itself will creat the MozNDEFRecord to
> compare or process the data.

OK, yeah. If you're just iterating over the TypedArray to compare it with known values, using Cu.cloneInto should be fine.
Whiteboard: [perf-reviewed]
Encounter a problem and would like to know if there is anyone could help me
Following is the code I wrote in marionette testcase

http://dxr.mozilla.org/mozilla-central/source/dom/webidl/MozNDEFRecord.webidl

let ndef = [new MozNDEFRecord(tnf, type, id, payload)];
let clone = Cu.cloneInto(ndef, window, {wrapReflectors:true});

log("clone ndef = " + clone);           //(1)Output clone ndef = [object MozNDEFRecord]
clone.forEach(function(record, index) {
  log("clone[index] = " + clone[index]);//(2)Output clone[index] = [object MozNDEFRecord]
  log("record = " + record);            //(3)Output record = [object XrayWrapper [object MozNDEFRecord]]
});

I am confused about output(3) because in that case the object is still Xray Wrapped, originally I though output(2)&(3) will be the same.
So this code is running in the scope of some global that is not |window|? If so, then cloning it into |window| will (potentially) give you an XrayWrapper when observing the clone from the first global, depending on the principals of the two globals.
Attached patch bug 1028037 fix v1 (obsolete) — Splinter Review
Hi,
 Could you help check if this patch is reasonable ? thanks !
Attachment #8449936 - Flags: feedback?(bobbyholley)
Attachment #8449936 - Attachment is obsolete: true
Attachment #8449936 - Flags: feedback?(bobbyholley)
Attached patch bug 1028037 fix v2 (obsolete) — Splinter Review
Attachment #8449940 - Flags: feedback?(bobbyholley)
Comment on attachment 8449936 [details] [diff] [review]
bug 1028037 fix v1

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

Passing a MozNDEFRecord to Cu.cloneInto isn't going to help. The MozNDEFRecord is an opaque object as far as the structured clone is concerned - so with { wrapReflector: true }, you'll just get a cross-compartment reference to it.

The stuff you want to clone is the .payload or the .id of a given record. Note that if this is just for test code, you can also just do Cu.waiveXrays(record).payload and access it that way.

::: dom/nfc/tests/marionette/head.js
@@ +6,3 @@
>  let pendingEmulatorCmdCount = 0;
>  
> +let Promise = Cu.import("resource://gre/modules/Promise.jsm").Promise;

Loading things like Promise.jsm via SpecialPowers wrappers is going to give you really weird behavior. You should just use DOM promises instead and kill this line.
Attachment #8449936 - Flags: feedback-
Comment on attachment 8449940 [details] [diff] [review]
bug 1028037 fix v2

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

Looks like we raced there. Same comments apply to this patch.
Attachment #8449940 - Flags: feedback?(bobbyholley) → feedback-
Attached patch Bug 1028037 fix v3 (obsolete) — Splinter Review
Thanks for your comment !
Update patch to use Cu.waiveXrays to unwrap Xrays.
About the Promise, I will create another bug to fix it.
Attachment #8449940 - Attachment is obsolete: true
Attachment #8449984 - Flags: review?(bobbyholley)
Attachment #8449984 - Flags: review?(bobbyholley) → review+
Add r=bholley
Attachment #8449984 - Attachment is obsolete: true
Attachment #8450004 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/e257340ac8dc
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.