1084902 - TypeError: "to" is read-only in disabled profiler tests

Status: RESOLVED FIXED

(DevTools :: Performance Tools (Profiler/Timeline), defect)

Description

[Sami Jaktholm]

Steps to reproduce:
1) Enable browser_profiler_console-record-09.js and browser_profiler_data-massaging-01.js in browser/devtools/profiler/test/browser.ini
2) Run ./mach mochitest-devtools browser/devtools/profiler/test/browser_profiler_console-record-09.js browser/devtools/profiler/test/browser_profiler_data-massaging-01.js from the fx-team tree root
3) Observe an apparent hang and following exception in the terminal:

A coding exception was thrown and uncaught in a Task.
Full message: TypeError: "to" is read-only
Full stack: ProfilerConnection.prototype._request@resource:///modules/devtools/profiler/shared.js:171:7
ProfilerFront.prototype.startRecording<@resource:///modules/devtools/profiler/shared.js:373:13
test<@chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_data-massaging-01.js:17:9

Comment 1

[Victor Porof [:vporof][:vp]]

This probably started failing because the tests are disabled. Yet another reason to fix bug 1047124...

Comment 2

[Tom Tromey :tromey]

If I enable all the profiler tests by editing the appropriate browser.ini,
and then run them, I see quite a few fails like this:

343 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_data-massaging-01.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
369 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_data-samples.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
384 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_gecko-pref-changed.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
483 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_profile-deselection.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
540 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_recording-selected-01.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
554 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_recording-selected-02.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
568 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_recording-selected-03.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
582 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_recording-utils.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
596 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_recordings-clear.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
610 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_recordings-io-01.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
866 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-front-02.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
878 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-front-03.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
913 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-front-05.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
954 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_simple-record-01.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
968 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_simple-record-02.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
982 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_simple-record-03.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
998 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_sudden-deactivation-01.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1014 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_sudden-deactivation-02.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1028 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_tabbed-browser-01.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1042 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_tabbed-browser-02.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1056 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_tabbed-browser-03.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1070 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_tabbed-browser-add-remove-01.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1084 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_tabbed-browser-add-remove-02.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1611 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_data-massaging-01.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1614 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_data-samples.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1617 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_gecko-pref-changed.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1620 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_profile-deselection.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1623 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_recording-selected-01.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1626 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_recording-selected-02.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1629 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_recording-selected-03.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1632 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_recording-utils.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1635 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_recordings-clear.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1638 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_recordings-io-01.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1641 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-front-02.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1644 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-front-03.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1647 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_shared-front-05.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1650 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_simple-record-01.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1653 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_simple-record-02.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1656 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_simple-record-03.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1659 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_sudden-deactivation-01.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1662 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_sudden-deactivation-02.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1665 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_tabbed-browser-01.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1668 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_tabbed-browser-02.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1671 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_tabbed-browser-03.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1674 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_tabbed-browser-add-remove-01.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only
1677 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/profiler/test/browser_profiler_tabbed-browser-add-remove-02.js | A promise chain failed to handle a rejection:  - at resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/shared.js:171 - TypeError: "to" is read-only

Comment 3

[Tom Tromey :tromey]

FWIW this hack at least lets the tests run so that I can debug other things
(I also applied the obvious change to browser/devtools/profiler/test/browser.ini
to re-enable all the tests.)

I couldn't quickly find what makes "to" read-only.

diff --git a/browser/devtools/profiler/utils/shared.js b/browser/devtools/profiler/utils/shared.js
index 69c00b2..32700bd 100644
--- a/browser/devtools/profiler/utils/shared.js
+++ b/browser/devtools/profiler/utils/shared.js
@@ -370,7 +370,12 @@ ProfilerFront.prototype = {
     // for all toolboxes and interacts with the whole platform, so we don't want
     // to affect other clients by stopping (or restarting) it.
     if (!isActive) {
-      yield this._request("profiler", "startProfiler", this._customProfilerOptions);
+      let localOptions = {
+        entries: this._customProfilerOptions.entries,
+        interval: this._customProfilerOptions.interval,
+        features: this._customProfilerOptions.features
+      };
+      yield this._request("profiler", "startProfiler", localOptions);
       this._profilingStartTime = 0;
       this.emit("profiler-activated");
     } else {

Comment 4

[Sami Jaktholm]

When ProfilerFront._customProfilerOptions it passed to ProfilerConnection._request for the first time following code path is taken with the object reference:
-> shared.js:173: this._client.request(ref, ...)
   -> http://dxr.mozilla.org/mozilla-central/source/browser/devtools/profiler/utils/shared.js#173
-> dbg-client.jsm:676: _pendingRequests.push(ref)
-> dbg-client.jsm:677: _sendRequests()
   -> http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/client/dbg-client.jsm#677
-> dbg-client.jsm:817: _transport.send(ref)
   -> http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/client/dbg-client.jsm#817
-> transport.js:536: deepFreeze(ref)
   -> http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/transport/transport.js#536

Now the original ProfilerConnection._customProfilerOptions object is completely frozen and trying to use it again causes the exception.

Comment 5

[Tom Tromey :tromey]

Thanks for finding that.

I am quite curious to know how you found it.  Was it by simply reading the code?  Or some other means?

Comment 6

[Sami Jaktholm]

(In reply to Tom Tromey :tromey from comment #5)
> I am quite curious to know how you found it.  Was it by simply reading the
> code?  Or some other means?

First I grep'd the code to figure out which code-path was causing the issue. After seeing this._customProfilerOptions to be passed to the offending function I figured out that that was the issue.

After that I used the chrome debugger to follow the object around to figure out where it was frozen. When I stepped over this._client.request(data, deferred.resolve) in shared.js:173 the debugger showed the object had been frozen (a little lock appeared next to it in the scope variable list).

On the next attempt I stepped in to the method and repeated the same process. That lead me to transport.js _send method that contained a call to deepFreeze which sounded about right. Then it was just a matter of taking a quick look at the code to confirm that the method did what its name implied.

Comment 7

[Tom Tromey :tromey]

Attachment: copy _customProfilerOptions before sending request

After looking a bit, I think my original hack is reasonable.  It's
probably the best spot to do the copying, as it is the only spot that
knows the structure of the object to copy (it is opaque to lower
layers).  Making new objects is what other front objects seem to do as
well.

I added a comment to explain why this was needed.

Note that this patch (without the comment) and a browser.ini patch to
re-enable the profiler tests were part of the try build for bug 1047124:

https://tbpl.mozilla.org/?tree=Try&rev=2d82d889a2a0

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8517680 [details] [diff] [review]
copy _customProfilerOptions before sending request

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

::: browser/devtools/profiler/utils/shared.js
@@ +371,5 @@
>      // to affect other clients by stopping (or restarting) it.
>      if (!isActive) {
> +      // Make a copy of the options, because eventually _request wants
> +      // to freeze the packet.
> +      let localOptions = {

I believe you could also do:

let localOptions = Cu.cloneInto(this._customProfilerOptions, {});

to copy the object without needing to list properties individually.

Comment 9

[Tom Tromey :tromey]

Attachment: copy _customProfilerOptions before sending request

Thank you very much, this works nicely.

Comment 10

[Patrick Brosset <:pbro>]

Comment on attachment 8518192 [details] [diff] [review]
copy _customProfilerOptions before sending request

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

Looks reasonable to me. Sorry for the delay for reviewing this simple patch.
Do you intend to re-enable the disabled test in this bug too? Or is there a follow-up bug on file already?

Comment 11

[Tom Tromey :tromey]

(In reply to Patrick Brosset [:pbrosset] [:patrick] from comment #10)

> Looks reasonable to me. Sorry for the delay for reviewing this simple patch.

No worries!

> Do you intend to re-enable the disabled test in this bug too? Or is there a
> follow-up bug on file already?

See the dependencies of bug 973974 (this is one of them).
I think these all should be fixed before re-enabling the tests.
The undiagnosed ones are in bug 1094920.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/a6dc5b5496fb

Comment 13

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

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