Closed Bug 1225219 Opened 9 years ago Closed 9 years ago

Allow ErrorResult objects to be cloned

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox43 --- wontfix
firefox44 --- fixed
firefox45 --- fixed

People

(Reporter: bkelly, Assigned: bkelly)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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: nobody → bkelly
Status: NEW → ASSIGNED
I tested the ErrorWithMessage case locally.
Attachment #8688120 - Flags: review?(bzbarsky)
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+
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 on attachment 8689011 [details] [diff] [review]
Implement ErrorResult::CloneTo(). r=bz

r=me
Attachment #8689011 - Flags: review?(bzbarsky) → review+
Flags: needinfo?(bkelly)
https://hg.mozilla.org/mozilla-central/rev/1b5636e31365
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
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+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: