Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(30)

Issue 2841443005: [Bindings] Create and use V8 context snapshots (Closed)

Created:
3 years, 8 months ago by peria
Modified:
3 years, 4 months ago
CC:
apavlov+blink_chromium.org, blink-reviews, blink-reviews-api_chromium.org, blink-reviews-bindings_chromium.org, blink-worker-reviews_chromium.org, caseq+blink_chromium.org, cbentzel+watch_chromium.org, chromium-reviews, darin-cc_chromium.org, devtools-reviews_chromium.org, dglazkov+blink, falken+watch_chromium.org, horo+watch_chromium.org, jam, kinuko+watch, kinuko+worker_chromium.org, kozyatinskiy+blink_chromium.org, lushnikov+blink_chromium.org, net-reviews_chromium.org, pfeldman+blink_chromium.org, shimazu+worker_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Create and use V8 context snapshots. This CL does two things. 1. In compile time, creates a snapshot file, which consists of V8 contexts. 2. Creates v8::Context from the snapshot in LocalWindowProxy::CreateContext(). We expect this speeds up context creation for 3 times faster on Android. Detailed information is described in the design doc [1]. [1] Design doc: https://docs.google.com/document/d/1jpQQX0piaxcHJPWakp_Kr_03g5Gnma5h5-Kdlqu7jVQ/edit#heading=h.k6iklq6rvd30 Test expectations are changed due to http://crbug.com/705364 BUG=588893, 617892, 705364 Review-Url: https://codereview.chromium.org/2841443005 Cr-Commit-Position: refs/heads/master@{#490329} Committed: https://chromium.googlesource.com/chromium/src/+/54afe29f866e4c3878242de7a78941fc363791e9

Patch Set 1 : . #

Total comments: 32

Patch Set 2 : Fix tests and introduce RuntimeEnabled flag #

Total comments: 2

Patch Set 3 : Fix some behaviors #

Total comments: 124

Patch Set 4 : Work for some comments #

Total comments: 58

Patch Set 5 : Work for most comments #

Total comments: 84

Patch Set 6 : Work for most comments #

Total comments: 14

Patch Set 7 : Work for all comments #

Total comments: 48

Patch Set 8 : Support runtime feature on templates #

Total comments: 10

Patch Set 9 : Work for yuki's comments #

Total comments: 34

Patch Set 10 : Move snapshot maker to tools/ and work for comments #

Total comments: 11

Patch Set 11 : Work for comments #

Total comments: 2

Patch Set 12 : Work for comments #

Patch Set 13 : Disable on ChromeOS and reduce size of table #

Total comments: 150

Patch Set 14 : Work for comments #

Total comments: 52

Patch Set 15 : Work for other than DOMWrapperWorld #

Patch Set 16 : Work for nits and libc fix? #

Total comments: 8

Patch Set 17 : Update GN files #

Patch Set 18 : Clean up #

Total comments: 4

Patch Set 19 : Update manifest #

Patch Set 20 : Take it down on Android #

Patch Set 21 : Fix libc++ error #

Patch Set 22 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1676 lines, -162 lines) Patch
M build/config/features.gni View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +2 lines, -1 line 0 comments Download
M chrome/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 3 chunks +3 lines, -0 lines 0 comments Download
M content/app/content_main_runner.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 4 chunks +34 lines, -18 lines 0 comments Download
M content/public/app/mojo/content_renderer_manifest.json View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +7 lines, -1 line 0 comments Download
M content/public/common/content_descriptor_keys.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -0 lines 0 comments Download
M content/public/common/content_descriptor_keys.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +3 lines, -1 line 0 comments Download
M content/shell/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 2 chunks +10 lines, -0 lines 0 comments Download
M extensions/shell/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +2 lines, -0 lines 0 comments Download
M gin/isolate_holder.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 5 chunks +46 lines, -35 lines 0 comments Download
M gin/public/isolate_holder.h View 1 2 3 4 5 6 7 2 chunks +6 lines, -2 lines 0 comments Download
M gin/v8_initializer.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +14 lines, -0 lines 0 comments Download
M gin/v8_initializer.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 11 chunks +70 lines, -27 lines 0 comments Download
M third_party/WebKit/LayoutTests/inspector-protocol/emulation/device-emulation-320-2x-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/inspector-protocol/emulation/device-emulation-320-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/inspector-protocol/emulation/device-emulation-320-only-viewport-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/inspector-protocol/emulation/device-emulation-980-2x-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/inspector-protocol/emulation/device-emulation-980-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/inspector-protocol/emulation/device-emulation-980-only-viewport-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/inspector-protocol/emulation/device-emulation-controls-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/inspector-protocol/emulation/device-emulation-dw-2x-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/inspector-protocol/emulation/device-emulation-dw-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/inspector-protocol/emulation/device-emulation-initial-scale-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/inspector-protocol/emulation/device-emulation-insets-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/inspector-protocol/emulation/device-emulation-none-2x-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/inspector-protocol/emulation/device-emulation-none-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/inspector-protocol/emulation/device-emulation-restore-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/inspector-protocol/emulation/device-emulation-small-dw-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/inspector-protocol/emulation/device-emulation-small-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/bindings/bindings.gni View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 8 chunks +52 lines, -28 lines 0 comments Download
A third_party/WebKit/Source/bindings/core/v8/V8ContextSnapshot.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +79 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/bindings/core/v8/V8ContextSnapshot.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +501 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/V8DOMConfiguration.cpp View 1 2 3 4 5 6 7 8 2 chunks +13 lines, -10 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/V8Initializer.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 4 chunks +20 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/bindings/modules/v8/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 2 chunks +35 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/bindings/modules/v8/V8ContextSnapshotExternalReferences.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +29 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/bindings/modules/v8/generated.gni View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/bindings/modules/v8/v8.gni View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +1 line, -0 lines 0 comments Download
A third_party/WebKit/Source/bindings/scripts/generate_v8_context_snapshot_external_references.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +223 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/bindings/templates/external_reference_table.cpp.tmpl View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +125 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/bindings/templates/templates.gni View 1 2 3 4 5 2 chunks +3 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/controller/BlinkInitializer.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 2 chunks +3 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/exported/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +1 line, -0 lines 0 comments Download
A third_party/WebKit/Source/core/exported/WebV8ContextSnapshot.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +16 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/workers/WorkerBackingThread.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 2 chunks +6 lines, -1 line 0 comments Download
M third_party/WebKit/Source/modules/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5 View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/bindings/DOMWrapperWorld.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/bindings/DOMWrapperWorld.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/bindings/ScriptWrappable.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/platform/bindings/V8PerContextData.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/platform/bindings/V8PerIsolateData.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 6 chunks +41 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/platform/bindings/V8PerIsolateData.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 6 chunks +71 lines, -7 lines 0 comments Download
M third_party/WebKit/public/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/public/platform/Platform.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +7 lines, -0 lines 0 comments Download
A third_party/WebKit/public/web/WebV8ContextSnapshot.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +23 lines, -0 lines 0 comments Download
A tools/v8_context_snapshot/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 1 chunk +97 lines, -0 lines 0 comments Download
A tools/v8_context_snapshot/DEPS View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +7 lines, -0 lines 0 comments Download
A tools/v8_context_snapshot/OWNERS View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +6 lines, -0 lines 0 comments Download
A tools/v8_context_snapshot/run.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +15 lines, -0 lines 0 comments Download
A tools/v8_context_snapshot/v8_context_snapshot_generator.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +69 lines, -0 lines 0 comments Download

Messages

Total messages: 205 (125 generated)
peria
Not for land for now. PS1 still have build failures on 3 trybots, but I ...
3 years, 7 months ago (2017-04-28 04:28:53 UTC) #9
Yuki
I've not yet reviewed the entire CL, but I'm about running out the time. So, ...
3 years, 7 months ago (2017-04-28 13:48:28 UTC) #14
peria
Not address Shiino-san's comments yet, but please take another look. I believe PS2 passes all ...
3 years, 7 months ago (2017-05-01 09:12:43 UTC) #17
Michael Lippautz
https://codereview.chromium.org/2841443005/diff/60001/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp File third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp (right): https://codereview.chromium.org/2841443005/diff/60001/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp#newcode232 third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp:232: per_isolate_data->GetScriptWrappableVisitor()->RegisterV8Reference( Line 232 is not needed. SetWrapper will take ...
3 years, 7 months ago (2017-05-04 17:30:23 UTC) #21
peria
https://codereview.chromium.org/2841443005/diff/60001/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp File third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp (right): https://codereview.chromium.org/2841443005/diff/60001/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp#newcode232 third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp:232: per_isolate_data->GetScriptWrappableVisitor()->RegisterV8Reference( On 2017/05/04 17:30:23, Michael Lippautz wrote: > Line ...
3 years, 7 months ago (2017-05-09 03:19:15 UTC) #25
Yuki
Still reviewing, but let me send another intermediate review comments. There seem a lot of ...
3 years, 7 months ago (2017-05-10 08:58:43 UTC) #28
Yuki
I'll review the following two files next week. bindings/core/v8/V8SnapshotCreator.cpp bindings/templates/snapshot.cpp.tmpl I think that I've taken ...
3 years, 7 months ago (2017-05-12 15:20:11 UTC) #29
haraken
(I'm sorry that I didn't have time to look at this CL (and may not ...
3 years, 7 months ago (2017-05-15 09:29:57 UTC) #30
Yuki
https://codereview.chromium.org/2841443005/diff/80001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotCreator.cpp File third_party/WebKit/Source/bindings/core/v8/V8SnapshotCreator.cpp (right): https://codereview.chromium.org/2841443005/diff/80001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotCreator.cpp#newcode22 third_party/WebKit/Source/bindings/core/v8/V8SnapshotCreator.cpp:22: bool g_taking_snapshot = false; Put a comment when this ...
3 years, 7 months ago (2017-05-15 09:37:29 UTC) #31
Yuki
https://codereview.chromium.org/2841443005/diff/80001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotCreator.h File third_party/WebKit/Source/bindings/core/v8/V8SnapshotCreator.h (right): https://codereview.chromium.org/2841443005/diff/80001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotCreator.h#newcode18 third_party/WebKit/Source/bindings/core/v8/V8SnapshotCreator.h:18: enum FieldType { s/FieldType/InternalFieldType/? "FieldType" would be a little ...
3 years, 7 months ago (2017-05-15 09:39:35 UTC) #32
haraken
Sorry about the review delay! I finally got a chance to look at this. Here ...
3 years, 7 months ago (2017-05-20 19:10:03 UTC) #37
peria
Thank you for your reviews. Worked for most comments other than naming consensuses. PTAL. https://codereview.chromium.org/2841443005/diff/40001/gin/shell/gin_prepare_main.cc ...
3 years, 6 months ago (2017-05-30 08:25:44 UTC) #38
Yuki
Comments from an intermediate review. I'm still reviewing. Will send the rest later. https://codereview.chromium.org/2841443005/diff/120001/chrome/BUILD.gn File ...
3 years, 6 months ago (2017-05-30 14:35:58 UTC) #43
haraken
(I've posted all major comments in #37 and you've already addressed them, so let me ...
3 years, 6 months ago (2017-05-30 14:52:08 UTC) #44
peria
Worked for most comments, including renames. I think remaining concerns are - to rename V8SnapshotCreator ...
3 years, 6 months ago (2017-06-01 08:33:34 UTC) #47
Yuki
Will review PS6 tomorrow. https://codereview.chromium.org/2841443005/diff/140001/third_party/WebKit/Source/platform/bindings/V8PerIsolateData.cpp File third_party/WebKit/Source/platform/bindings/V8PerIsolateData.cpp (right): https://codereview.chromium.org/2841443005/diff/140001/third_party/WebKit/Source/platform/bindings/V8PerIsolateData.cpp#newcode99 third_party/WebKit/Source/platform/bindings/V8PerIsolateData.cpp:99: g_main_thread_per_isolate_data = this; Let's avoid ...
3 years, 6 months ago (2017-06-01 14:15:51 UTC) #50
peria
Updated. PTAL. Discussed with Yuki again and I'll update to install runtime features on templates ...
3 years, 6 months ago (2017-06-20 10:20:15 UTC) #53
Yuki
https://codereview.chromium.org/2841443005/diff/160001/gin/BUILD.gn File gin/BUILD.gn (right): https://codereview.chromium.org/2841443005/diff/160001/gin/BUILD.gn#newcode142 gin/BUILD.gn:142: # functions into one if the function signatures and ...
3 years, 6 months ago (2017-06-20 14:20:11 UTC) #56
peria
PTL. Worked for comments on other than gin/BUILD.gn. The main change is to install runtime ...
3 years, 6 months ago (2017-06-21 07:19:17 UTC) #57
Yuki
Major points were discussed offline. Only minor comments here. https://codereview.chromium.org/2841443005/diff/180001/gin/isolate_holder.cc File gin/isolate_holder.cc (right): https://codereview.chromium.org/2841443005/diff/180001/gin/isolate_holder.cc#newcode85 gin/isolate_holder.cc:85: ...
3 years, 6 months ago (2017-06-21 09:23:21 UTC) #62
peria
methods to install runtime enabled features on templates are willing to land in another patch ...
3 years, 6 months ago (2017-06-23 02:22:05 UTC) #66
Yuki
https://codereview.chromium.org/2841443005/diff/200001/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp File third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp (right): https://codereview.chromium.org/2841443005/diff/200001/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp#newcode273 third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp:273: // and Window. I think that now we don't ...
3 years, 6 months ago (2017-06-23 15:20:45 UTC) #69
peria
https://codereview.chromium.org/2841443005/diff/160001/gin/BUILD.gn File gin/BUILD.gn (right): https://codereview.chromium.org/2841443005/diff/160001/gin/BUILD.gn#newcode147 gin/BUILD.gn:147: } else if (is_posix && !is_mac) { On 2017/06/23 ...
3 years, 5 months ago (2017-06-27 09:52:37 UTC) #72
Yuki
https://codereview.chromium.org/2841443005/diff/240001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp File third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp (right): https://codereview.chromium.org/2841443005/diff/240001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp#newcode166 third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp:166: CHECK(!context.IsEmpty()); This CHECK is meaningless after ToLocalChecked(). https://codereview.chromium.org/2841443005/diff/240001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp#newcode194 third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp:194: ...
3 years, 5 months ago (2017-06-27 12:43:48 UTC) #80
Yuki
https://codereview.chromium.org/2841443005/diff/240001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp File third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp (right): https://codereview.chromium.org/2841443005/diff/240001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp#newcode207 third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp:207: if (world.IsMainWorld()) { nit: Can we do an early-exit? ...
3 years, 5 months ago (2017-06-27 12:43:49 UTC) #81
peria
https://codereview.chromium.org/2841443005/diff/240001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp File third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp (right): https://codereview.chromium.org/2841443005/diff/240001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp#newcode166 third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp:166: CHECK(!context.IsEmpty()); On 2017/06/27 12:43:47, Yuki wrote: > This CHECK ...
3 years, 5 months ago (2017-06-28 03:02:44 UTC) #82
Yuki
LGTM with comments. https://codereview.chromium.org/2841443005/diff/260001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp File third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp (right): https://codereview.chromium.org/2841443005/diff/260001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp#newcode264 third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp:264: // Update the install function for ...
3 years, 5 months ago (2017-06-28 06:38:32 UTC) #87
haraken
(I'll take a look at this CL by next Tue.)
3 years, 5 months ago (2017-06-28 07:19:50 UTC) #88
peria
On 2017/06/28 07:19:50, haraken wrote: > (I'll take a look at this CL by next ...
3 years, 5 months ago (2017-06-28 07:39:24 UTC) #89
haraken
On 2017/06/28 07:39:24, peria wrote: > On 2017/06/28 07:19:50, haraken wrote: > > (I'll take ...
3 years, 5 months ago (2017-06-28 07:42:29 UTC) #90
peria
On 2017/06/28 07:42:29, haraken wrote: > On 2017/06/28 07:39:24, peria wrote: > > On 2017/06/28 ...
3 years, 5 months ago (2017-06-28 15:41:08 UTC) #91
haraken
Let me take a look at V8SnapshotUtil.cpp tomorrow. All the comments are about nits. Looks ...
3 years, 5 months ago (2017-07-04 15:04:21 UTC) #105
haraken
This CL is mixing "Blink V8 snapshot", "V8 snapshot" and "V8 context snapshot". I'd like ...
3 years, 5 months ago (2017-07-04 15:06:17 UTC) #106
haraken
Here is a final round of comments! https://codereview.chromium.org/2841443005/diff/320001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp File third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp (right): https://codereview.chromium.org/2841443005/diff/320001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp#newcode36 third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp:36: v8::Context::Scope scope(context); ...
3 years, 5 months ago (2017-07-06 13:15:54 UTC) #108
Yuki
https://codereview.chromium.org/2841443005/diff/320001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp File third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp (right): https://codereview.chromium.org/2841443005/diff/320001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp#newcode144 third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp:144: struct DataForDeserializer { On 2017/07/06 13:15:53, haraken wrote: > ...
3 years, 5 months ago (2017-07-06 14:12:28 UTC) #109
Yuki
https://codereview.chromium.org/2841443005/diff/320001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp File third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp (right): https://codereview.chromium.org/2841443005/diff/320001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp#newcode144 third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp:144: struct DataForDeserializer { On 2017/07/06 14:12:28, Yuki wrote: > ...
3 years, 5 months ago (2017-07-06 14:15:25 UTC) #110
haraken
https://codereview.chromium.org/2841443005/diff/320001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp File third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp (right): https://codereview.chromium.org/2841443005/diff/320001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp#newcode222 third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp:222: if (!world.IsMainWorld()) { On 2017/07/06 14:12:28, Yuki wrote: > ...
3 years, 5 months ago (2017-07-06 14:22:13 UTC) #111
Yuki
https://codereview.chromium.org/2841443005/diff/320001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp File third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp (right): https://codereview.chromium.org/2841443005/diff/320001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp#newcode482 third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp:482: V8PerIsolateData::From(isolate)->ClearPersistentsForV8Snapshot(); On 2017/07/06 14:22:13, haraken wrote: > On 2017/07/06 ...
3 years, 5 months ago (2017-07-06 14:40:09 UTC) #112
peria
Worked for most comments. Some experimental works are on going. Thank you Yuki for answering ...
3 years, 5 months ago (2017-07-07 06:22:04 UTC) #115
haraken
https://codereview.chromium.org/2841443005/diff/320001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp File third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp (right): https://codereview.chromium.org/2841443005/diff/320001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp#newcode346 third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp:346: char* data = new char[size]; On 2017/07/07 06:22:00, peria ...
3 years, 5 months ago (2017-07-07 06:59:11 UTC) #116
peria
PTAL. https://codereview.chromium.org/2841443005/diff/320001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp File third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp (right): https://codereview.chromium.org/2841443005/diff/320001/third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp#newcode242 third_party/WebKit/Source/bindings/core/v8/V8SnapshotUtil.cpp:242: v8::Local<v8::Function> interface = data->ConstructorForType(type); On 2017/07/06 13:15:53, haraken ...
3 years, 5 months ago (2017-07-10 03:39:13 UTC) #120
peria
+Nico in R. Could you take a look, especially for GN changes?
3 years, 5 months ago (2017-07-10 03:40:32 UTC) #124
haraken
https://codereview.chromium.org/2841443005/diff/320001/third_party/WebKit/Source/platform/bindings/V8PerIsolateData.cpp File third_party/WebKit/Source/platform/bindings/V8PerIsolateData.cpp (right): https://codereview.chromium.org/2841443005/diff/320001/third_party/WebKit/Source/platform/bindings/V8PerIsolateData.cpp#newcode67 third_party/WebKit/Source/platform/bindings/V8PerIsolateData.cpp:67: : nullptr), On 2017/07/10 03:39:13, peria wrote: > On ...
3 years, 5 months ago (2017-07-10 04:58:46 UTC) #128
peria
https://codereview.chromium.org/2841443005/diff/320001/third_party/WebKit/Source/platform/bindings/V8PerIsolateData.cpp File third_party/WebKit/Source/platform/bindings/V8PerIsolateData.cpp (right): https://codereview.chromium.org/2841443005/diff/320001/third_party/WebKit/Source/platform/bindings/V8PerIsolateData.cpp#newcode67 third_party/WebKit/Source/platform/bindings/V8PerIsolateData.cpp:67: : nullptr), On 2017/07/10 04:58:46, haraken wrote: > On ...
3 years, 5 months ago (2017-07-10 05:37:56 UTC) #129
haraken
LGTM with comments! https://codereview.chromium.org/2841443005/diff/360001/gin/v8_initializer.h File gin/v8_initializer.h (right): https://codereview.chromium.org/2841443005/diff/360001/gin/v8_initializer.h#newcode84 gin/v8_initializer.h:84: static void GetV8ContextSnapshotData(const char** snapshot_data_out, Add ...
3 years, 5 months ago (2017-07-10 06:52:09 UTC) #130
peria
https://codereview.chromium.org/2841443005/diff/360001/gin/v8_initializer.h File gin/v8_initializer.h (right): https://codereview.chromium.org/2841443005/diff/360001/gin/v8_initializer.h#newcode84 gin/v8_initializer.h:84: static void GetV8ContextSnapshotData(const char** snapshot_data_out, On 2017/07/10 06:52:08, haraken ...
3 years, 5 months ago (2017-07-10 10:18:44 UTC) #132
peria
https://codereview.chromium.org/2841443005/diff/360001/third_party/WebKit/Source/bindings/core/v8/V8ContextSnapshot.cpp File third_party/WebKit/Source/bindings/core/v8/V8ContextSnapshot.cpp (right): https://codereview.chromium.org/2841443005/diff/360001/third_party/WebKit/Source/bindings/core/v8/V8ContextSnapshot.cpp#newcode276 third_party/WebKit/Source/bindings/core/v8/V8ContextSnapshot.cpp:276: DOMWrapperWorld::Create(isolate, DOMWrapperWorld::WorldType::kTesting); On 2017/07/10 10:18:43, peria wrote: > On ...
3 years, 5 months ago (2017-07-11 06:54:32 UTC) #136
haraken
LGTM! https://codereview.chromium.org/2841443005/diff/360001/third_party/WebKit/Source/bindings/core/v8/V8ContextSnapshot.cpp File third_party/WebKit/Source/bindings/core/v8/V8ContextSnapshot.cpp (right): https://codereview.chromium.org/2841443005/diff/360001/third_party/WebKit/Source/bindings/core/v8/V8ContextSnapshot.cpp#newcode276 third_party/WebKit/Source/bindings/core/v8/V8ContextSnapshot.cpp:276: DOMWrapperWorld::Create(isolate, DOMWrapperWorld::WorldType::kTesting); On 2017/07/11 06:54:31, peria wrote: > ...
3 years, 5 months ago (2017-07-11 09:05:12 UTC) #137
peria
+R jochen for gin/ jam for content/ Please take a look
3 years, 5 months ago (2017-07-11 10:05:03 UTC) #141
Nico
The gn changes generally lgtm. Was there an intent-to-implement thread for this somewhere? What's the ...
3 years, 5 months ago (2017-07-11 18:59:40 UTC) #144
peria
Thank you for reviewing. I updated the document, and the disk usage will grow for ...
3 years, 5 months ago (2017-07-12 02:38:37 UTC) #145
peria
+R dcheng@ for content/public/app/mojo/content_renderer_manifest.json rkc@ for extensions/shell/BUILD.gn eroman@ for net/BUILD.gn brettw@ for url/BUILD.gn Could you ...
3 years, 5 months ago (2017-07-12 09:27:09 UTC) #151
dcheng
https://codereview.chromium.org/2841443005/diff/440001/content/public/app/mojo/content_renderer_manifest.json File content/public/app/mojo/content_renderer_manifest.json (right): https://codereview.chromium.org/2841443005/diff/440001/content/public/app/mojo/content_renderer_manifest.json#newcode81 content/public/app/mojo/content_renderer_manifest.json:81: "path": "v8_context_snapshot.bin", Nit: please use spaces https://codereview.chromium.org/2841443005/diff/440001/content/public/app/mojo/content_renderer_manifest.json#newcode85 content/public/app/mojo/content_renderer_manifest.json:85: "path": ...
3 years, 5 months ago (2017-07-12 09:55:09 UTC) #152
peria
https://codereview.chromium.org/2841443005/diff/440001/content/public/app/mojo/content_renderer_manifest.json File content/public/app/mojo/content_renderer_manifest.json (right): https://codereview.chromium.org/2841443005/diff/440001/content/public/app/mojo/content_renderer_manifest.json#newcode81 content/public/app/mojo/content_renderer_manifest.json:81: "path": "v8_context_snapshot.bin", On 2017/07/12 09:55:09, dcheng wrote: > Nit: ...
3 years, 5 months ago (2017-07-12 10:30:37 UTC) #153
Nico
On Jul 11, 2017 10:38 PM, <peria@chromium.org> wrote: Thank you for reviewing. I updated the ...
3 years, 5 months ago (2017-07-12 10:36:54 UTC) #156
Nico
On Jul 11, 2017 10:38 PM, <peria@chromium.org> wrote: Thank you for reviewing. I updated the ...
3 years, 5 months ago (2017-07-12 10:36:55 UTC) #157
dcheng
manifest change lgtm
3 years, 5 months ago (2017-07-12 21:17:41 UTC) #161
eroman
//net LGTM (although I didn't see the conclusion for why current_toolchain == default_toolchain needs to ...
3 years, 5 months ago (2017-07-12 22:01:34 UTC) #162
rkc
rs-lgtm for //extensions/shell/BUILD.gn
3 years, 5 months ago (2017-07-13 01:19:17 UTC) #163
haraken
As discussed in blink-dev@, let's disable the V8 context snapshot on Android at the moment.
3 years, 5 months ago (2017-07-13 01:21:16 UTC) #164
peria
On 2017/07/13 01:21:16, haraken wrote: > As discussed in blink-dev@, let's disable the V8 context ...
3 years, 5 months ago (2017-07-13 06:45:58 UTC) #171
peria
jam@, jochen@ Could you take a look? content/ and gin/ contain some important changes.
3 years, 5 months ago (2017-07-13 06:47:08 UTC) #172
jochen (gone - plz use gerrit)
gin lgtm
3 years, 5 months ago (2017-07-13 14:34:47 UTC) #175
peria
jam@ Could you take a look?
3 years, 5 months ago (2017-07-17 00:25:13 UTC) #176
peria
jochen@, Could you also review code under content/?
3 years, 5 months ago (2017-07-20 05:20:12 UTC) #177
haraken
Can you move this CL forward?
3 years, 4 months ago (2017-07-27 18:01:30 UTC) #178
peria
On 2017/07/27 18:01:30, haraken wrote: > Can you move this CL forward? Sure. I need ...
3 years, 4 months ago (2017-07-27 23:35:55 UTC) #179
peria
Kinuko-san, Could you review changes under content/?
3 years, 4 months ago (2017-07-27 23:36:54 UTC) #181
kinuko
On 2017/07/27 23:36:54, peria wrote: > Kinuko-san, > Could you review changes under content/? lgtm
3 years, 4 months ago (2017-07-28 01:27:30 UTC) #182
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2841443005/520001
3 years, 4 months ago (2017-07-28 04:34:34 UTC) #185
peria
Thank you, all reviewers. Trying to land it.
3 years, 4 months ago (2017-07-28 04:35:07 UTC) #186
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/350068)
3 years, 4 months ago (2017-07-28 05:39:12 UTC) #188
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2841443005/520001
3 years, 4 months ago (2017-07-28 05:42:52 UTC) #190
commit-bot: I haz the power
Failed to commit the patch.
3 years, 4 months ago (2017-07-28 08:42:53 UTC) #195
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2841443005/520001
3 years, 4 months ago (2017-07-28 08:46:06 UTC) #197
commit-bot: I haz the power
Committed patchset #22 (id:520001) as https://chromium.googlesource.com/chromium/src/+/54afe29f866e4c3878242de7a78941fc363791e9
3 years, 4 months ago (2017-07-28 08:55:52 UTC) #201
blundell
A revert of this CL (patchset #22 id:520001) has been created in https://codereview.chromium.org/2989793003/ by blundell@chromium.org. ...
3 years, 4 months ago (2017-07-28 09:47:03 UTC) #202
blundell
On 2017/07/28 09:47:03, blundell wrote: > A revert of this CL (patchset #22 id:520001) has ...
3 years, 4 months ago (2017-07-28 09:55:20 UTC) #203
agrieve
On 2017/07/28 09:55:20, blundell wrote: > On 2017/07/28 09:47:03, blundell wrote: > > A revert ...
3 years, 4 months ago (2017-07-28 18:25:33 UTC) #204
peria
3 years, 4 months ago (2017-08-01 06:06:51 UTC) #205
Message was sent while issue was closed.
On 2017/07/28 18:25:33, agrieve wrote:
> Also worth noting that this registered as an 18kb apk size increase. Based on
> the change, this is probably expected, but thought I'd point it out in case
this
> is at all addressable.

Thank you for pointing it out.
It should be addressed in the patch, and I'll work for it on re-landing.

Powered by Google App Engine
This is Rietveld 408576698