Closed
Bug 1435483
Opened 7 years ago
Closed 7 years ago
Do some cleanup on our exception classes
Categories
(Core :: DOM: Core & HTML, enhancement)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla60
Tracking | Status | |
---|---|---|
firefox60 | --- | fixed |
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
(Blocks 1 open bug)
Details
Attachments
(23 files, 1 obsolete file)
There's a bunch of XPCOM goop we can remove here.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → bzbarsky
Assignee | ||
Comment 1•7 years ago
|
||
This code has never worked correctly. Bug 911258 landed on 2013-09-09 and
removed the initialize() method from XPConnect exceptions. This code landed
two days after that. If it's ever reached, it will just throw when calling the
nonexistent initialize() method.
MozReview-Commit-ID: FWpP1fLBIPW
Attachment #8948203 -
Flags: review?(poirot.alex)
Assignee | ||
Comment 2•7 years ago
|
||
Bobby, could you review the CanCreateInstance call removal carefully? I'm pretty sure we should be system-only there, but maybe I'm missing something?
Attachment #8948204 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 3•7 years ago
|
||
This interface is not usable from JS, because we don't expose initialize() in
the WebIDL bindings for Exception. And C++ doesn't use it.
MozReview-Commit-ID: LsIm4YA0YZE
Attachment #8948205 -
Flags: review?(kyle)
Assignee | ||
Comment 4•7 years ago
|
||
MozReview-Commit-ID: H1ZPg76xNyI
Attachment #8948206 -
Flags: review?(kyle)
Assignee | ||
Comment 5•7 years ago
|
||
MozReview-Commit-ID: D3uuehuDqOB
Attachment #8948207 -
Flags: review?(kyle)
Assignee | ||
Comment 6•7 years ago
|
||
MozReview-Commit-ID: 7aYg9kJhiab
Attachment #8948208 -
Flags: review?(kyle)
Assignee | ||
Comment 7•7 years ago
|
||
MozReview-Commit-ID: 7VJvDR0qD3G
Attachment #8948209 -
Flags: review?(kyle)
Assignee | ||
Comment 8•7 years ago
|
||
MozReview-Commit-ID: KpRyt21PF7W
Attachment #8948210 -
Flags: review?(kyle)
Assignee | ||
Comment 9•7 years ago
|
||
MozReview-Commit-ID: ADxO2A8nkel
Attachment #8948211 -
Flags: review?(kyle)
Assignee | ||
Comment 10•7 years ago
|
||
MozReview-Commit-ID: 8pdMDFHWlVt
Attachment #8948212 -
Flags: review?(kyle)
Assignee | ||
Comment 11•7 years ago
|
||
MozReview-Commit-ID: KLSzUuWt45x
Attachment #8948213 -
Flags: review?(kyle)
Assignee | ||
Comment 12•7 years ago
|
||
MozReview-Commit-ID: CTCawPvw6VZ
Attachment #8948214 -
Flags: review?(kyle)
Assignee | ||
Comment 13•7 years ago
|
||
MozReview-Commit-ID: CXnwjeHoGRm
Attachment #8948215 -
Flags: review?(kyle)
Assignee | ||
Comment 14•7 years ago
|
||
MozReview-Commit-ID: 6JN7UvkhPgl
Attachment #8948216 -
Flags: review?(kyle)
Assignee | ||
Comment 15•7 years ago
|
||
It's doing casts that are bogus. We can do the same thing (only needed for
tests anyway) via a chromeonly API on DOMRequest.
MozReview-Commit-ID: 1FUPGMhBU3k
Attachment #8948217 -
Flags: review?(kyle)
Assignee | ||
Comment 16•7 years ago
|
||
nsIException is builtinclass in idl, so whatever code we had to handle
non-dom::Exception nsIExceptions is dead code.
MozReview-Commit-ID: 6VnqDWt0041
Attachment #8948218 -
Flags: review?(kyle)
Assignee | ||
Comment 17•7 years ago
|
||
MozReview-Commit-ID: GKzE812BfIF
Attachment #8948219 -
Flags: review?(kyle)
Assignee | ||
Comment 18•7 years ago
|
||
MozReview-Commit-ID: JArus2ddEsL
Attachment #8948220 -
Flags: review?(kyle)
Assignee | ||
Comment 19•7 years ago
|
||
MozReview-Commit-ID: AbVQ7IA4xqp
Attachment #8948221 -
Flags: review?(kyle)
Assignee | ||
Comment 20•7 years ago
|
||
MozReview-Commit-ID: 9cDzCeddOmh
Attachment #8948222 -
Flags: review?(kyle)
Assignee | ||
Comment 21•7 years ago
|
||
MozReview-Commit-ID: 17yuhlqzbr2
Attachment #8948223 -
Flags: review?(kyle)
Assignee | ||
Comment 22•7 years ago
|
||
Assignee | ||
Comment 23•7 years ago
|
||
Assignee | ||
Comment 24•7 years ago
|
||
MozReview-Commit-ID: GKzE812BfIF
Attachment #8948438 -
Flags: review?(kyle)
Assignee | ||
Updated•7 years ago
|
Attachment #8948219 -
Attachment is obsolete: true
Attachment #8948219 -
Flags: review?(kyle)
Comment 25•7 years ago
|
||
Comment on attachment 8948203 [details] [diff] [review]
part 1. Stop using nsIXPCException in devtools code
I'm not sure this code is still used, this was related to FXOS.
Attachment #8948203 -
Flags: review?(poirot.alex) → review+
Comment 26•7 years ago
|
||
(In reply to Alexandre Poirot [:ochameau] from comment #25)
> I'm not sure this code is still used, this was related to FXOS.
Created bug 1435791 to remove this code.
Updated•7 years ago
|
Attachment #8948205 -
Flags: review?(kyle) → review+
Updated•7 years ago
|
Attachment #8948206 -
Flags: review?(kyle) → review+
Updated•7 years ago
|
Attachment #8948207 -
Flags: review?(kyle) → review+
Updated•7 years ago
|
Attachment #8948208 -
Flags: review?(kyle) → review+
Comment 27•7 years ago
|
||
Comment on attachment 8948209 [details] [diff] [review]
part 7. Add an infallible "columnNumber" getter on nsIStackFrame
Review of attachment 8948209 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/bindings/Exceptions.cpp
@@ +479,5 @@
>
> return col;
> }
>
> +NS_IMETHODIMP JSStackFrame::GetColumnNumberXPCOM(JSContext* aCx,
nit: Might as well fix formatting while you're here?
Attachment #8948209 -
Flags: review?(kyle) → review+
Updated•7 years ago
|
Attachment #8948210 -
Flags: review?(kyle) → review+
Updated•7 years ago
|
Attachment #8948211 -
Flags: review?(kyle) → review+
Assignee | ||
Comment 28•7 years ago
|
||
> nit: Might as well fix formatting while you're here?
Done.
Updated•7 years ago
|
Attachment #8948212 -
Flags: review?(kyle) → review+
Updated•7 years ago
|
Attachment #8948213 -
Flags: review?(kyle) → review+
Updated•7 years ago
|
Attachment #8948214 -
Flags: review?(kyle) → review+
Updated•7 years ago
|
Attachment #8948215 -
Flags: review?(kyle) → review+
Updated•7 years ago
|
Attachment #8948216 -
Flags: review?(kyle) → review+
Updated•7 years ago
|
Attachment #8948217 -
Flags: review?(kyle) → review+
Updated•7 years ago
|
Attachment #8948218 -
Flags: review?(kyle) → review+
Updated•7 years ago
|
Attachment #8948220 -
Flags: review?(kyle) → review+
Updated•7 years ago
|
Attachment #8948221 -
Flags: review?(kyle) → review+
Updated•7 years ago
|
Attachment #8948222 -
Flags: review?(kyle) → review+
Comment 29•7 years ago
|
||
Comment on attachment 8948223 [details] [diff] [review]
part 21. Remove nsIException::ToString
Review of attachment 8948223 [details] [diff] [review]:
-----------------------------------------------------------------
::: xpcom/base/nsIException.idl
@@ +66,5 @@
> };
>
> +// This interface only exists because of all the JS consumers doing
> +// "instanceof Ci.nsIException". We should switch them to something else and
> +// get rid of it. C++ code should NOT use this; use mozilla::dom::Exception
nit: Do we have followup filed for this? If so, can we mention it here?
Attachment #8948223 -
Flags: review?(kyle) → review+
Updated•7 years ago
|
Attachment #8948438 -
Flags: review?(kyle) → review+
Assignee | ||
Comment 30•7 years ago
|
||
> nit: Do we have followup filed for this?
Not yet, but I can file and mention: bug 1435856.
Blocks: 1435856
Updated•7 years ago
|
Attachment #8948204 -
Flags: review?(bobbyholley) → review+
Comment 31•7 years ago
|
||
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/af047d35cc1f
part 1. Stop using nsIXPCException in devtools code. r=ochameau
https://hg.mozilla.org/integration/mozilla-inbound/rev/4798fb33e87f
part 2. Stop allowing creation of Exception objects via contract/CID. r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/432d38e26230
part 3. Remove nsIXPCException. r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/c916ed1fb22e
part 4. Remove always-true mInitialized member from Exception. r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/06e2c91ca04f
part 5. Add an infallible "filename" getter on nsIStackFrame. r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/feec23374328
part 6. Add an infallible "lineNumber" getter on nsIStackFrame. r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/5376dbad3062
part 7. Add an infallible "columnNumber" getter on nsIStackFrame. r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/2cb9916edac9
part 8. Add an infallible "asyncCause" getter on nsIStackFrame. r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c7e9384e092
part 9. Add an infallible "name" getter on nsIStackFrame. r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/4065bdc6d415
part 10. Add infallible "asyncCaller" and "caller" getters on nsIStackFrame. r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/233b3c3c8c4b
part 11. Add an infallible "formattedStack" getter on nsIStackFrame. r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/b7d20b7d9230
part 12. Add an infallible "toString" method on nsIStackFrame. r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/6c6988ae4688
part 13. Remove nsIException::GetName. r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/43b00825c08c
part 14. Remove nsIException::GetFilename/GetLineNumber/GetColumnNumber. r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/c8e4cf95eb66
part 15. Remove nsIDOMRequestService::FireDetailedError. r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/1cc1b9042bb3
part 16. Switch to using dom::Exception, not nsIException, in C++ code. r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/3dc0da16efed
part 17. Remove nsIException::GetMessageMoz. r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/bd57d71589c2
part 18. Remove nsIException::GetResult. r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/bbad188127c1
part 19. Remove nsIException::GetLocation. r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8eaa9b9c2ce
part 20. Remove nsIException::GetData. r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/aba564b1d5ec
part 21. Remove nsIException::ToString. r=qdot
Comment 32•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/af047d35cc1f
https://hg.mozilla.org/mozilla-central/rev/4798fb33e87f
https://hg.mozilla.org/mozilla-central/rev/432d38e26230
https://hg.mozilla.org/mozilla-central/rev/c916ed1fb22e
https://hg.mozilla.org/mozilla-central/rev/06e2c91ca04f
https://hg.mozilla.org/mozilla-central/rev/feec23374328
https://hg.mozilla.org/mozilla-central/rev/5376dbad3062
https://hg.mozilla.org/mozilla-central/rev/2cb9916edac9
https://hg.mozilla.org/mozilla-central/rev/3c7e9384e092
https://hg.mozilla.org/mozilla-central/rev/4065bdc6d415
https://hg.mozilla.org/mozilla-central/rev/233b3c3c8c4b
https://hg.mozilla.org/mozilla-central/rev/b7d20b7d9230
https://hg.mozilla.org/mozilla-central/rev/6c6988ae4688
https://hg.mozilla.org/mozilla-central/rev/43b00825c08c
https://hg.mozilla.org/mozilla-central/rev/c8e4cf95eb66
https://hg.mozilla.org/mozilla-central/rev/1cc1b9042bb3
https://hg.mozilla.org/mozilla-central/rev/3dc0da16efed
https://hg.mozilla.org/mozilla-central/rev/bd57d71589c2
https://hg.mozilla.org/mozilla-central/rev/bbad188127c1
https://hg.mozilla.org/mozilla-central/rev/a8eaa9b9c2ce
https://hg.mozilla.org/mozilla-central/rev/aba564b1d5ec
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•