|
|
Description[d8] Fix ArrayBuffer memory leaks in d8 introduced by commit 96635558.
BUG=686338
R=cbruni@chromium.org,jbroman@chromium.org
Review-Url: https://codereview.chromium.org/2662193002
Cr-Commit-Position: refs/heads/master@{#42937}
Committed: https://chromium.googlesource.com/v8/v8/+/67fc083aae6e10ef054dca403db4a19671b3a1f9
Patch Set 1 #
Total comments: 2
Patch Set 2 : comment #Messages
Total messages: 24 (15 generated)
The CQ bit was checked by binji@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== [d8] Fix ArrayBuffer memory leaks in d8 introduced by commit 96635558. BUG=686388 R=cbruni@chromium.org,jbroman@chromium.org ========== to ========== [d8] Fix ArrayBuffer memory leaks in d8 introduced by commit 96635558. BUG=686338 R=cbruni@chromium.org,jbroman@chromium.org ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
ping :)
lgtm https://codereview.chromium.org/2662193002/diff/1/src/d8.cc File src/d8.cc (right): https://codereview.chromium.org/2662193002/diff/1/src/d8.cc#newcode2583 src/d8.cc:2583: typename T::Contents MaybeExternalize(Local<T>* array_buffer) { Why not just Local<T>? You don't seem to modify this parameter, even though it looks like an out-variable.
https://codereview.chromium.org/2662193002/diff/1/src/d8.cc File src/d8.cc (right): https://codereview.chromium.org/2662193002/diff/1/src/d8.cc#newcode2583 src/d8.cc:2583: typename T::Contents MaybeExternalize(Local<T>* array_buffer) { On 2017/02/02 00:14:32, jbroman wrote: > Why not just Local<T>? You don't seem to modify this parameter, even though it > looks like an out-variable. Done.
The CQ bit was checked by binji@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by binji@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jbroman@chromium.org Link to the patchset: https://codereview.chromium.org/2662193002/#ps20001 (title: "comment")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
No L-G-T-M from a valid reviewer yet. CQ run can only be started by full committers or once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer, _not_ a full super star committer. Committers are members of the group "project-v8-committers". Note that this has nothing to do with OWNERS files.
lgtm
The CQ bit was checked by binji@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 20001, "attempt_start_ts": 1486167463767660, "parent_rev": "de20581ca37011d6ef9490b6c3f25c1a594ab200", "commit_rev": "67fc083aae6e10ef054dca403db4a19671b3a1f9"}
Message was sent while issue was closed.
Description was changed from ========== [d8] Fix ArrayBuffer memory leaks in d8 introduced by commit 96635558. BUG=686338 R=cbruni@chromium.org,jbroman@chromium.org ========== to ========== [d8] Fix ArrayBuffer memory leaks in d8 introduced by commit 96635558. BUG=686338 R=cbruni@chromium.org,jbroman@chromium.org Review-Url: https://codereview.chromium.org/2662193002 Cr-Commit-Position: refs/heads/master@{#42937} Committed: https://chromium.googlesource.com/v8/v8/+/67fc083aae6e10ef054dca403db4a19671b... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/v8/v8/+/67fc083aae6e10ef054dca403db4a19671b... |