Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2685743002:
V8ValueConverterImpl: Use V8 to copy from array buffers. (Closed)
|
Created:
3 years, 10 months ago by jbroman Modified:
3 years, 10 months ago Reviewers:
jochen (gone - plz use gerrit) CC:
chromium-reviews, darin-cc_chromium.org, jam, Devlin Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionV8ValueConverterImpl: Use V8 to copy from array buffers.
It is no longer necessary to route through Blink and externalize them.
This has the side effect of not requiring all of the Blink machinery to be
set up in order to convert binary values from V8.
Review-Url: https://codereview.chromium.org/2685743002
Cr-Commit-Position: refs/heads/master@{#448997}
Committed: https://chromium.googlesource.com/chromium/src/+/54311d097280fedbf6d8e3cb310898461d1a3df1
Patch Set 1 #Patch Set 2 : also the reverse direction, and a simple unit test #Patch Set 3 : plus a DCHECK that matches the one in WebArrayBufferConverter::toV8Value #Patch Set 4 : fix dcheck #
Total comments: 1
Patch Set 5 : correct dcheck, again :( #
Messages
Total messages: 26 (21 generated)
The CQ bit was checked by jbroman@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 checked by jbroman@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 checked by jbroman@chromium.org to run a CQ dry run
The CQ bit was checked by jbroman@chromium.org to run a CQ dry run
Patchset #3 (id:2) has been deleted
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: Try jobs failed on following builders: linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
The CQ bit was checked by jbroman@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 checked by jbroman@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...
jbroman@chromium.org changed reviewers: + jochen@chromium.org
This is shorter, simpler, and doesn't force the buffer to be externalized (which isn't necessary to copy out of it). It also means that this can be tested in unit tests suites that don't necessarily have Blink initialized, like content_unittests (this patch) and extensions_unittests (the motivation for this patch). https://codereview.chromium.org/2685743002/diff/70001/content/child/v8_value_... File content/child/v8_value_converter_impl.cc (right): https://codereview.chromium.org/2685743002/diff/70001/content/child/v8_value_... content/child/v8_value_converter_impl.cc:339: DCHECK(creation_context == isolate->GetCurrentContext()); This matches the DCHECK that already exists in WebArrayBufferConverter.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm please format the CL description to be less than 80c
Description was changed from ========== V8ValueConverterImpl: Use V8 to copy from array buffers, without routing through Blink and making it externalize them. This has the side effect of not requiring all of the Blink machinery to be set up in order to convert binary values from V8. ========== to ========== V8ValueConverterImpl: Use V8 to copy from array buffers. It is no longer necessary to route through Blink and externalize them. This has the side effect of not requiring all of the Blink machinery to be set up in order to convert binary values from V8. ==========
The CQ bit was checked by jbroman@chromium.org
On 2017/02/08 at 08:57:24, jochen wrote: > lgtm > > please format the CL description to be less than 80c Done.
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": 90001, "attempt_start_ts": 1486566390303820,
"parent_rev": "21c93b0db86d1d7aa881a7f08778c6f20a935406", "commit_rev":
"54311d097280fedbf6d8e3cb310898461d1a3df1"}
Message was sent while issue was closed.
Description was changed from ========== V8ValueConverterImpl: Use V8 to copy from array buffers. It is no longer necessary to route through Blink and externalize them. This has the side effect of not requiring all of the Blink machinery to be set up in order to convert binary values from V8. ========== to ========== V8ValueConverterImpl: Use V8 to copy from array buffers. It is no longer necessary to route through Blink and externalize them. This has the side effect of not requiring all of the Blink machinery to be set up in order to convert binary values from V8. Review-Url: https://codereview.chromium.org/2685743002 Cr-Commit-Position: refs/heads/master@{#448997} Committed: https://chromium.googlesource.com/chromium/src/+/54311d097280fedbf6d8e3cb3108... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:90001) as https://chromium.googlesource.com/chromium/src/+/54311d097280fedbf6d8e3cb3108... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
