1267021 - Use fallible allocation and move semantics for push events

Status: RESOLVED FIXED

(Core :: DOM: Push Subscriptions, defect)

Description

[Lina Butler [:lina]]

Attachment: fallibleAlloc.patch

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.

Comment 1

[William Chen [:wchen]]

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).

Comment 2

[Lina Butler [:lina]]

(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`.

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/662a8ccb9a3e

Comment 4

[Lina Butler [:lina]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=75f33e5fc03c

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/662a8ccb9a3e