Move buffer argument from StartIncrementalEncoding to FinishIncrementalEncoding.

RESOLVED FIXED in Firefox 55

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: nbp, Assigned: nbp)

Tracking

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Currently the buffer given as argument to the XDRIncrementalEncoder is only used to append data into when running the linearize method.  Having this argument held by the XDRIncrementalEncoder pose some issues as we keep a reference to a dangling pointer.

By moving it to the FinishIncrementalEncoding / linearize functions, we no longer have to question this clean-up & lifetime issues.  Also, this simplify the nsJSUtils interface, as we do not have to pass the TranscodeBuffer as argument of the ExecutionContext anymore. (which simplifies Bug 1364117)
(Assignee)

Comment 1

2 years ago
This patch changes the JS API added to use the XDR incremental encoder, and
update its uses, such as the code added in Bug 900784.
Attachment #8870039 - Flags: review?(shu)
Attachment #8870039 - Flags: review?(mrbkap)
Comment on attachment 8870039 [details] [diff] [review]
Move buffer argument from JS::StartIncrementalEncoding to JS::FinishIncrementalEncoding.

Review of attachment 8870039 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/script/ScriptLoader.cpp
@@ +2182,5 @@
>    TRACE_FOR_TEST_NONE(aRequest->mElement, "scriptloader_bytecode_saved");
>  }
>  
>  void
>  ScriptLoader::GiveUpBytecodeEncoding()

I'd love for the duplication of the draining while loop below to be pulled out, but I'm not sure if it's worth it. It's not required for this bug, but maybe something to think about.
Attachment #8870039 - Flags: review?(mrbkap) → review+

Updated

2 years ago
Attachment #8870039 - Flags: review?(shu) → review+
(Assignee)

Updated

2 years ago
Blocks: 1367515

Comment 3

2 years ago
Pushed by npierron@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3967231c7021
Move buffer argument from JS::StartIncrementalEncoding to JS::FinishIncrementalEncoding. r=mrbkap,shu

Comment 4

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b5ef50a79731
Add explicit keyword in front of XDRIncrementalEncoder constructor. r=me on a CLOSED TREE

Comment 5

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3967231c7021
https://hg.mozilla.org/mozilla-central/rev/b5ef50a79731
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.