1452981 - Remove qsObjectHelper

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P3)

Comment 1

[Peter Van der Beken [:peterv]]

Attachment: Use ToJSValue instead of WrapObject if we know we have a nsWrapperCache v1

Comment 2

[Peter Van der Beken [:peterv]]

Attachment: Use ToJSValue instead of WrapObject if we know we have a nsWrapperCache v1

Comment 3

[Peter Van der Beken [:peterv]]

Attachment: Remove qsObjectHelper v1

Comment 4

[Peter Van der Beken [:peterv]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=813027177da859868ec8a74f25e9750fe0e2b639

Comment 5

[Peter Van der Beken [:peterv]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=00beb2336745f339039de73c60c1670f3bc01348

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8966631 [details] [diff] [review]
Use ToJSValue instead of WrapObject if we know we have a nsWrapperCache v1

>@@ -126,33 +126,29 @@ WindowNamedPropertiesHandler::getOwnProp
>+  if (!document->ResolveName(aCx, str, &v, rv)) {
>+    return !rv.Failed();

This doesn't seem right.  If rv failed, we'll return false here.  But we're not setting an exception on the JSContext, so it'll become an uncatchable exception?

Or is the point that rv can only fail here if an exception is already on the JSContext?  If so, we should document that somewhere...  In any case, examining the return value before we checked rv doesn't make sense; it's not guaranteed to mean anything.  So this part should look like so:

  ErrorResult rv;
  bool found = document->ResolveName(aCx, str, &v, rv);
  if (rv.MaybeSetPendingException(cx)) {
    return false;
  }

  if (found) {
    FillPropertyDescriptor(aDesc, aProxy, 0, v);
  }
  return true;

or so.

>+nsHTMLDocument::ResolveName(JSContext* aCx, const nsAString& aName,
>+        aError.Throw(NS_ERROR_OUT_OF_MEMORY);

  aError.NoteJSContextException();

both places in that method.

r=me with those fixed.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8966633 [details] [diff] [review]
Remove qsObjectHelper v1

r=me

Comment 8

[Peter Van der Beken [:peterv]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/02178545f255e96f95540dba7668c6a7dda079fd
Bug 1452981 - Use ToJSValue instead of WrapObject if we know we have a nsWrapperCache. r=bz.

https://hg.mozilla.org/integration/mozilla-inbound/rev/80abe3305b24b7f2c251ac973a287275a488428f
Bug 1452981 - Remove qsObjectHelper. r=bz.

Comment 9

[Dorel Luca [:dluca]]

Backed out 8 changesets (bug 1453011, bug 1452981, bug 1146316) For xpcshell and mochitest failures on multiple files. CLOSED TREE

Log of the failure:
https://treeherder.mozilla.org/logviewer.html#?job_id=174810370&repo=mozilla-inbound&lineNumber=1577

09:36:27     INFO -  TEST-PASS | devtools/client/performance/test/unit/test_jit-graph-data.js | took 12963ms
09:36:27     INFO -  TEST-START | devtools/client/performance/test/unit/test_tree-model-06.js
09:36:31     INFO -  TEST-PASS | devtools/client/performance/test/unit/test_tree-model-02.js | took 11871ms
09:36:31     INFO -  TEST-START | devtools/client/performance/test/unit/test_tree-model-07.js
09:36:31     INFO -  mozcrash Saved minidump as /Users/cltbld/tasks/task_1524241879/build/blobber_upload_dir/F9603462-C1C9-4C82-BB9E-546B04DCD948.dmp
09:36:31     INFO -  mozcrash Saved app info as /Users/cltbld/tasks/task_1524241879/build/blobber_upload_dir/F9603462-C1C9-4C82-BB9E-546B04DCD948.extra
09:36:31  WARNING -  PROCESS-CRASH | xpcshell-remote.ini:browser/components/extensions/test/xpcshell/test_ext_geckoProfiler_control.js | application crashed [@ ProcessExecutableMemory::release()]
09:36:31     INFO -  Crash dump filename: /var/folders/x8/56fhxqzs7rx3spfwdg5l65t800000w/T/xpc-other-Wm7KED/F9603462-C1C9-4C82-BB9E-546B04DCD948.dmp
09:36:31     INFO -  Operating system: Mac OS X
09:36:31     INFO -                    10.10.5 14F27
09:36:31     INFO -  CPU: amd64
09:36:31     INFO -       family 6 model 69 stepping 1
09:36:31     INFO -       4 CPUs
09:36:31     INFO -  GPU: UNKNOWN
09:36:31     INFO -  Crash reason:  EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
09:36:31     INFO -  Crash address: 0x0
09:36:31     INFO -  Process uptime: 25 seconds
09:36:31     INFO -  Thread 0 (crashed)
09:36:31     INFO -   0  XUL!ProcessExecutableMemory::release() [ProcessExecutableMemory.cpp:033299f2733900eb0da9b5b4d814aea39ce552d4 : 492 + 0x0]
09:36:31     INFO -      rax = 0x0000000000000000   rdx = 0x00007fff75f5c1f8
09:36:31     INFO -      rcx = 0x0000000000000000   rbx = 0x0000000117fb6690
09:36:31     INFO -      rsi = 0x0003c3000003c300   rdi = 0x0003c2000003c303
09:36:31     INFO -      rbp = 0x00007fff50502dd0   rsp = 0x00007fff50502dc0
09:36:31     INFO -       r8 = 0x00007fff50502d70    r9 = 0x00007fff75f86300
09:36:31     INFO -      r10 = 0x0000000000000000   r11 = 0x0000000000000246
09:36:31     INFO -      r12 = 0x000000011860a9d0   r13 = 0x00007fff50502e30
09:36:31     INFO -      r14 = 0x0000000117fcd690   r15 = 0x00007fff50502e08
09:36:31     INFO -      rip = 0x0000000114af2026
09:36:31     INFO -      Found by: given as instruction pointer in context


Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/30ed797c2454b1f5f259f1c26f85bd7a62380ef5

Push of the failures:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=033299f2733900eb0da9b5b4d814aea39ce552d4

Comment 10

[Peter Van der Beken [:peterv]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f4d5d3d50fb5c9798f24cdad517844dff6f926b3
Bug 1452981 - Use ToJSValue instead of WrapObject if we know we have a nsWrapperCache. r=bz.

https://hg.mozilla.org/integration/mozilla-inbound/rev/078000699e1c8bd1ae4cd30cc0580f835a618e2c
Bug 1452981 - Remove qsObjectHelper. r=bz.

Comment 11

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/f4d5d3d50fb5
https://hg.mozilla.org/mozilla-central/rev/078000699e1c