Closed
Bug 1225219
Opened 9 years ago
Closed 9 years ago
Allow ErrorResult objects to be cloned
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla45
People
(Reporter: bkelly, Assigned: bkelly)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
2.67 KB,
patch
|
bzbarsky
:
review+
ritu
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
In bug 1217367 we need to report a single ErrorResult to multiple consumers. Since ErrorResults are depletable, that means we need to clone the value to each. Current API plan is ErrorResult::CloneTo(ErrorResult& aDest).
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•9 years ago
|
||
I tested the ErrorWithMessage case locally.
Attachment #8688120 -
Flags: review?(bzbarsky)
Comment 2•9 years ago
|
||
Comment on attachment 8688120 [details] [diff] [review] Implement ErrorResult::CloneTo(). r=bz Please document what the method actually does. There's no reason not to make this work with JS exceptions, right? Just make sure that CloneTo() copies the mMightHaveUnreportedJSException state and then if IsJSException() you can aRv.ThrowJSException() using the thread-default JSContext. r=me with those fixed.
Attachment #8688120 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 3•9 years ago
|
||
Add a comment to ErrorResult.h to document the call and support for JS exceptions in the cpp. Asking for re-review since I don't trust my jsapi knowledge for handling the JS exceptions.
Attachment #8688120 -
Attachment is obsolete: true
Attachment #8689011 -
Flags: review?(bzbarsky)
Comment 4•9 years ago
|
||
Comment on attachment 8689011 [details] [diff] [review] Implement ErrorResult::CloneTo(). r=bz r=me
Attachment #8689011 -
Flags: review?(bzbarsky) → review+
Backed out for build bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=17457460&repo=mozilla-inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/6d9cf8dd2ed8
Flags: needinfo?(bkelly)
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(bkelly)
Comment 9•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1b5636e31365
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Assignee | ||
Updated•9 years ago
|
status-firefox43:
--- → wontfix
status-firefox44:
--- → affected
Assignee | ||
Comment 10•9 years ago
|
||
Comment on attachment 8689011 [details] [diff] [review] Implement ErrorResult::CloneTo(). r=bz Note, please uplift the m-c version of the patch. There was a small change made there to fix opt builds. Approval Request Comment [Feature/regressing bug #]: Service workers. [User impact if declined]: We need this in order to uplift service worker bug 1217367. [Describe test coverage new/current, TreeHerder]: Test coverage provided in bug 1217637. [Risks and why]: Minimal. While this bug defines code in dom/bindings, its only called by service worker code today. No other features will be affected by the uplift. [String/UUID change made/needed]: None.
Attachment #8689011 -
Flags: approval-mozilla-aurora?
Comment on attachment 8689011 [details] [diff] [review] Implement ErrorResult::CloneTo(). r=bz Approved. Please note Ben's request on the patch to land the one from m-c.
Attachment #8689011 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 12•9 years ago
|
||
remote: https://hg.mozilla.org/releases/mozilla-aurora/rev/940121320e6e
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•