Previewing should be worker friendly

RESOLVED FIXED in Firefox 67

Status

defect
P2
normal
RESOLVED FIXED
4 months ago
21 days ago

People

(Reporter: jlast, Assigned: bhackett)

Tracking

(Regressed 1 bug)

unspecified
Firefox 67
Dependency tree / graph

Firefox Tracking Flags

(firefox67 fixed)

Details

Attachments

(2 attachments)

Reporter

Description

4 months ago

ObjectActor.prototype.grip is currently failing because Cu is not available in the worker context.

Priority: -- → P2
Reporter

Comment 2

3 months ago

Notes from slack

there's this kind of weird logic when we look at typed arrays to get the length while skipping over any overridden own 'length' property on the typed array

there are a couple problems here imo
1) the tab shouldn't crash! the wrapping code should throw instead of taking down the process
2) this logic seems unnecessary. if someone wants to override 'length' to do something weird, it seems like the debugger should respect that when displaying the typed array's contents
the offending logic is in getArrayLength in devtools/server/actors/object/utils.js
Assignee

Comment 3

3 months ago
Posted patch patchSplinter Review

This patch fixes previewing in workers for Array, TypedArray, Set, and Map. WeakSet and WeakMap are still broken --- they rely on some ChromeUtils interfaces which ought to be available in workers, but I haven't figured out how to actually access them here. Other previewers should work as expected. I'll try to write a test for these changes before landing.

Assignee: nobody → bhackett1024
Attachment #9046901 - Flags: data-review?(lsmyth)
Assignee

Updated

3 months ago
Attachment #9046901 - Flags: data-review?(lsmyth) → review?(lsmyth)
Attachment #9046901 - Flags: review?(lsmyth) → review+

Comment 4

3 months ago
Pushed by bhackett@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/09f23a363606
Fix previewing in worker threads for several builtin classes, r=lsmyth.

Comment 6

3 months ago
Pushed by bhackett@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4047bcadec73
Fix previewing in worker threads for several builtin classes, r=lsmyth.

Comment 7

3 months ago
bugherder
Status: NEW → RESOLVED
Last Resolved: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67

Backed out changeset 4047bcadec73 (Bug 1522244) for mochitest failure at devtools/client/debugger/new/test/mochitest/browser_dbg-worker-scopes.js a=backout

Push with failures: https://hg.mozilla.org/integration/mozilla-inbound/rev/4047bcadec73

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=231330045&repo=mozilla-inbound&lineNumber=24220
and https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=231329937&repo=mozilla-inbound&lineNumber=24442

21:15:55 INFO - TEST-PASS | devtools/client/debugger/new/test/mochitest/browser_dbg-worker-scopes.js | "two" -
21:15:55 INFO - Leaving test bound
21:15:55 INFO - Buffered messages logged at 13:15:55
21:15:55 INFO - Removing tab.
21:15:55 INFO - Waiting for event: 'TabClose' on [object XULElement].
21:15:55 INFO - Got event: 'TabClose' on [object XULElement].
21:15:55 INFO - Tab removed and finished closing
21:15:55 INFO - Buffered messages finished
21:15:55 INFO - TEST-UNEXPECTED-FAIL | devtools/client/debugger/new/test/mochitest/browser_dbg-worker-scopes.js | A promise chain failed to handle a rejection: Current state is running - stack: updateWorkers/<@resource://devtools/client/debugger/new/src/actions/debuggee.js:11:5
21:15:55 INFO - asyncthunk/</</<@resource://devtools/client/debugger/new/src/actions/utils/middleware/thunk.js:21:45
21:15:55 INFO - bindActionCreator/<@resource://devtools/client/shared/vendor/redux.js:644:12
21:15:55 INFO - workerListChanged@resource://devtools/client/debugger/new/src/client/firefox/events.js:89:11
21:15:55 INFO - emit@resource://devtools/shared/event-emitter.js:186:24
21:15:55 INFO - emit@resource://devtools/shared/event-emitter.js:267:18
21:15:55 INFO - onPacket@resource://devtools/shared/protocol.js:1424:13
21:15:55 INFO - onPacket@resource://devtools/shared/client/debugger-client.js:601:13
21:15:55 INFO - send/<@resource://devtools/shared/transport/local-transport.js:64:23
21:15:55 INFO - exports.makeInfallible/<@resource://devtools/shared/ThreadSafeDevToolsUtils.js:109:22
21:15:55 INFO - DevToolsUtils.executeSoon
exports.executeSoon@resource://devtools/shared/DevToolsUtils.js:55:21
21:15:55 INFO - send@resource://devtools/shared/transport/local-transport.js:58:21
21:15:55 INFO - send@resource://devtools/server/main.js:1023:20
21:15:55 INFO - receiveMessage@resource://devtools/shared/transport/child-transport.js:66:16
21:15:55 INFO - MessageListener.receiveMessage*_addListener@resource://devtools/shared/transport/child-transport.js:40:14
21:15:55 INFO - ready@resource://devtools/shared/transport/child-transport.js:57:10
21:15:55 INFO - connectToFrame/</onActorCreated<@resource://devtools/server/main.js:730:24
21:15:55 INFO - exports.makeInfallible/<@resource://devtools/shared/ThreadSafeDevToolsUtils.js:109:22
21:15:55 INFO - MessageListener.receiveMessagetrackMessageManager@resource://devtools/server/main.js:612:14
21:15:55 INFO - connectToFrame/<@resource://devtools/server/main.js:825:7
21:15:55 INFO - connectToFrame@resource://devtools/server/main.js:601:12
21:15:55 INFO - connect@resource://devtools/server/actors/targets/frame-proxy.js:57:36
21:15:55 INFO - async
BrowserTabList.prototype._getActorForBrowser@resource://devtools/server/actors/webbrowser.js:313:16

Comment 9

3 months ago
Backout by dvarga@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/e3762382f790
Backed out changeset 4047bcadec73 for mochitest failure at devtools/client/debugger/new/test/mochitest/browser_dbg-worker-scopes.js a=backout
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Firefox 67 → ---

Comment 10

3 months ago
Pushed by bhackett@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/faddaf06f46c
Fix previewing in worker threads for several builtin classes, r=lsmyth.

Comment 11

3 months ago
bugherder
Status: REOPENED → RESOLVED
Last Resolved: 3 months ago3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
Assignee

Updated

3 months ago
Flags: needinfo?(bhackett1024)

I see WeakMap/WeakSet were mentioned here but I don't see it tracked so I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1533202, feel free to dup if it already exists though.

I don't think

object.proto.proto.getOwnPropertyDescriptor("length").get

is reliable. The [[Prototype]] of the typed array could have been altered.
Same for callPropertyOnObject.

Regressions: 1547320
You need to log in before you can comment on or make changes to this bug.