Closed
Bug 1267021
Opened 8 years ago
Closed 8 years ago
Use fallible allocation and move semantics for push events
Categories
(Core :: DOM: Push Subscriptions, defect)
Core
DOM: Push Subscriptions
Tracking
()
RESOLVED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: lina, Assigned: lina)
Details
(Whiteboard: btpp-active)
Attachments
(1 file)
8.56 KB,
patch
|
wchen
:
review+
|
Details | Diff | Splinter Review |
Some trivial refactoring to occupy myself on the train. It looks like we can use fallible allocation and a move when passing push message data to the `PushMessageEvent` constructor. I also de-duped the nsTArray conversion methods while I was there.
Attachment #8744664 -
Flags: review?(wchen)
Comment 1•8 years ago
|
||
Comment on attachment 8744664 [details] [diff] [review] fallibleAlloc.patch Review of attachment 8744664 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/push/PushUtil.cpp @@ +10,5 @@ > /* static */ bool > PushUtil::CopyArrayBufferToArray(const ArrayBuffer& aBuffer, > nsTArray<uint8_t>& aArray) > { > + MOZ_ASSERT(aArray.IsEmpty()); This function and the next one correctly handles non-empty arrays so the assertion isn't really valid. If you want to make it a requirement that aArray is empty you can use InsertElementAt instead of SetLength + ReplaceElementAt similar to what ExtractBytesFromArrayBuffer is doing. That way we save calling the destructor on all the elements in the array before copying the data (although it might be optimized away since the array contains a fundamental data type).
Attachment #8744664 -
Flags: review?(wchen) → review+
Assignee | ||
Comment 2•8 years ago
|
||
(In reply to William Chen [:wchen] from comment #1) > If you want to make it a requirement that aArray is empty you can use > InsertElementAt instead of SetLength + ReplaceElementAt similar to what > ExtractBytesFromArrayBuffer is doing. That way we save calling the > destructor on all the elements in the array before copying the data > (although it might be optimized away since the array contains a fundamental > data type). Thanks for catching that! Changed here and in `PushManager::CopySubscriptionKeyToArray`.
Assignee | ||
Comment 4•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=75f33e5fc03c
Comment 5•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/662a8ccb9a3e
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in
before you can comment on or make changes to this bug.
Description
•