1332245 - Move nsScriptError from js/xpconnect to dom/bindings.

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Tooru Fujisawa [:arai]]

Derived from bug 1283712.

Currently nsScriptError is stored in js/xpconnect, especially declared in js/xpconnect/src/xpcprivate.h.
bug 1283712 is about to add some more method and class (addNotes, and nsScriptErrorNote) there and don't want to make it mutable via XPIDL for simplicity.
dom/worker needs to mutate, so nsScriptError and nsIScriptError should be moved under dom/

Comment 1

[Tooru Fujisawa [:arai]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=89bc2c20994b1295b9f19833d46189e35ecfaf83

Comment 2

[Tooru Fujisawa [:arai]]

hmm, things moved to the new module isn't recognized on linux on try run, but works correctly on OSX locally...

Comment 3

[Boris Zbarsky [:bzbarsky]]

Please leave this stuff in nsLayoutModule.

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Move nsScriptError from js/xpconnect to dom/bindings.

Moved the following files from js/xpconnect/{src,idl} to dom/bindings
  * nsIScriptError.idl
  * nsScriptError.cpp
  * nsScriptErrorWithStack.cpp
and also changed coding style from SpiderMonkey to Gecko.

Also added the following file, separated from js/xpconnect/src/xpcprivate.h
  * dom/bindings/nsScriptError.h
it's published to mozilla/dom/nsScriptError.h and is included from several files
that were using nsScriptError via xpcprivate.h

CID/CONTRACTID things are embedded directly to layout/build/nsLayoutModule.cpp,
moved from XPCONNECT_{CIDENTRIES,CONTRACTS}

One question, what should the value of XPIDL_MODULE for nsIScriptError.idl be?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8828748 [details] [diff] [review]
Move nsScriptError from js/xpconnect to dom/bindings.

Please don't combine moves and coding style changes in a single changeset.  In fact, I'm not sure it's worth the noise of changing coding style at all.

If we _do_ change coding style, I'd like to see a diff -w for whatever changeset does that.

As for XPIDL_MODULE... I guess "dom_bindings", but I expect it really doesn't matter much.

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: Move nsScriptError from js/xpconnect to dom/bindings.

Okay, reverted the coding style change.
(nsScriptError.h is in Gecko style, since it's new file)

also changed XPIDL_MODULE from dom to dom_bindings, and added dom_bindings.xpt to package-manifest.in files.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b4e066498b49e841054ee9ac7358d81c9817ed94&selectedJob=70899136

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8829064 [details] [diff] [review]
Move nsScriptError from js/xpconnect to dom/bindings.

Sorry for the horrible lag here.  :(

> +++ b/b2g/installer/package-manifest.in

Please file a bug on comm-central to add this new xpt to im/installer/package-manifest.in, mail/installer/package-manifest.in, and suite/installer/package-manifest.in.  Not doing that second one will certainly break Thunderbird...

>+++ b/dom/bindings/moz.build
>@@ -29,16 +35,17 @@ EXPORTS.mozilla.dom += [
>+    'nsScriptError.h',

I don't think we should export this to mozilla/dom (for one thing, it's not in the mozilla::dom namespace).

Probably better to not export it at all, and add '/dom/bindings' to LOCAL_INCLUDES in js/xpconnect/src/moz.build and layout/build/moz.build.

>+++ b/dom/bindings/nsScriptError.cpp

>  * nsIScriptError implementation.  Defined here, lacking a JS-specific
>  * place to put XPCOM things.

The second sentence doesn't seem relevant anymore.

>+++ b/dom/bindings/nsScriptError.h

>+// Definition of nsScriptError, defined here because we lack a place to put
>+// XPCOM objects associated with the JavaScript engine.

Again, the second half of this comment doesn't look right anymore.

>+  // TODO - do something reasonable on getting null from these babies.

This comment should probably go away, here or in a followup.

r=me with the above nits fixed.

Comment 8

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fa90a8ad3133263b4933236de7ed78dab7bace46
Bug 1332245 - Move nsScriptError from js/xpconnect to dom/bindings. r=bz

Comment 9

[Phil Ringnalda (:philor)]

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