Closed
Bug 1387385
Opened 8 years ago
Closed 8 years ago
Remove usage of nsIJSON from Web Payments
Categories
(Core :: DOM: Web Payments, enhancement)
Core
DOM: Web Payments
Tracking
()
RESOLVED
FIXED
mozilla57
| Tracking | Status | |
|---|---|---|
| firefox57 | --- | fixed |
People
(Reporter: hsivonen, Assigned: edenchuang)
References
Details
(Whiteboard: [WP-MVP][M4])
Attachments
(1 file, 2 obsolete files)
|
3.84 KB,
patch
|
baku
:
review+
|
Details | Diff | Splinter Review |
https://searchfox.org/mozilla-central/source/dom/payments/PaymentRequestUtils.cpp depends on nsIJSON.
Note how nsIJSON.idl says: "Don't use this!"
Please remove the nsIJSON dependency so that nsIJSON can be removed from the code base.
| Assignee | ||
Comment 1•8 years ago
|
||
I will remove nsIJSON from payment implementation.
Assignee: nobody → echuang
| Assignee | ||
Updated•8 years ago
|
Status: NEW → ASSIGNED
| Assignee | ||
Comment 2•8 years ago
|
||
| Assignee | ||
Comment 3•8 years ago
|
||
| Assignee | ||
Comment 4•8 years ago
|
||
Remove all nsIJSON from PaymentRequestUtils.cpp
Attachment #8895297 -
Attachment is obsolete: true
Attachment #8899660 -
Flags: review?(amarchesini)
Updated•8 years ago
|
Whiteboard: [WP-MVP][M4]
Comment 5•8 years ago
|
||
Comment on attachment 8899660 [details] [diff] [review]
Bug 1387385 - Bug 1387385 - Remove all nsIJSON usage in PaymentRequestUtils.cpp. r?baku
Review of attachment 8899660 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/payments/PaymentRequestUtils.cpp
@@ +28,2 @@
> JS::RootedValue value(aCx, JS::ObjectValue(*aObject));
> + JS::HandleValue handleVal(&value);
This is wrong. You just need to pass &value to SerializeFromJSVal.
Attachment #8899660 -
Flags: review?(amarchesini) → review-
| Assignee | ||
Comment 6•8 years ago
|
||
Modify the patch according to comment 5, but got following compile error when passing &value to SerializeFromJSVal.
13:27.69 /Volumes/fxos_workspace/gecko/mozilla-central/dom/payments/PaymentRequestUtils.cpp:29:10: error: no matching function for call to 'SerializeFromJSVal'
13:27.70 return SerializeFromJSVal(aCx, &value, aSerializedObject);
13:27.70 ^~~~~~~~~~~~~~~~~~
13:27.70 /Volumes/fxos_workspace/gecko/mozilla-central/dom/payments/PaymentRequestUtils.h:22:1: note: candidate function not viable: no known conversion from 'JS::RootedValue *' (aka 'Rooted<JS::Value> *') to 'JS::HandleValue' (aka 'Handle<JS::Value>') for 2nd argument
I guess that we should pass "value" here not "&value".
Passing "value" makes both build and mochitest pass, so I use "value" in this patch.
Attachment #8899660 -
Attachment is obsolete: true
Attachment #8901011 -
Flags: review?(amarchesini)
Updated•8 years ago
|
Attachment #8901011 -
Flags: review?(amarchesini) → review+
| Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/80622cca7657
Remove all nsIJSON usage in PaymentRequestUtils.cpp. r=baku
Keywords: checkin-needed
Comment 8•8 years ago
|
||
| bugherder | ||
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•