Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1542623004:
Add documentation about Isolate, Context, World, Frames etc (Closed)
DescriptionAdd documentation about Isolate, Context, World, Frames etc
BUG=
Committed: https://crrev.com/b6f387262b5241d6eb278604b59a8527e7996ee6
Cr-Commit-Position: refs/heads/master@{#366959}
Patch Set 1 #
Total comments: 8
Patch Set 2 : #
Total comments: 14
Messages
Total messages: 13 (4 generated)
haraken@chromium.org changed reviewers: + jochen@chromium.org, yukishiino@chromium.org
PTAL
LGTM with comments. https://codereview.chromium.org/1542623004/diff/1/third_party/WebKit/Source/b... File third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md (right): https://codereview.chromium.org/1542623004/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:102: When V8 binding invokes JavaScript, V8 binding creates a new context Exactly speaking, this seems a little bit wrong. This reads like that we need to always create a new context just in order to invoke JavaScript. My understanding is that: - V8 binding creates a context per each frame. - V8 binding specifies a context when invoking JavaScript (or V8 requires a context in which the given JavaScript should run). Thus, it's possible that we re-use an existing context when V8 binding invokes JavaScript. https://codereview.chromium.org/1542623004/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:118: [entry settings object](https://html.spec.whatwg.org/#entry-settings-object) Let's link to the multipage version. https://html.spec.whatwg.org/multipage/webappapis.html#entry-settings-object https://codereview.chromium.org/1542623004/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:120: [incumbent settings object](https://html.spec.whatwg.org/#incumbent-settings-object) Ditto. https://html.spec.whatwg.org/multipage/webappapis.html#incumbent-settings-object https://codereview.chromium.org/1542623004/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:162: and N isolated worlds. An isolate world of a worker thread consists of s/An isolate world of/An isolate of/
The CQ bit was checked by haraken@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from yukishiino@chromium.org Link to the patchset: https://codereview.chromium.org/1542623004/#ps20001 (title: " ")
https://codereview.chromium.org/1542623004/diff/1/third_party/WebKit/Source/b... File third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md (right): https://codereview.chromium.org/1542623004/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:102: When V8 binding invokes JavaScript, V8 binding creates a new context On 2015/12/24 07:15:15, Yuki wrote: > Exactly speaking, this seems a little bit wrong. This reads like that we need > to always create a new context just in order to invoke JavaScript. > > My understanding is that: > - V8 binding creates a context per each frame. > - V8 binding specifies a context when invoking JavaScript > (or V8 requires a context in which the given JavaScript should run). > > Thus, it's possible that we re-use an existing context when V8 binding invokes > JavaScript. Reworded: creates a new context => enters a context https://codereview.chromium.org/1542623004/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:118: [entry settings object](https://html.spec.whatwg.org/#entry-settings-object) On 2015/12/24 07:15:15, Yuki wrote: > Let's link to the multipage version. > https://html.spec.whatwg.org/multipage/webappapis.html#entry-settings-object Done. https://codereview.chromium.org/1542623004/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:120: [incumbent settings object](https://html.spec.whatwg.org/#incumbent-settings-object) On 2015/12/24 07:15:15, Yuki wrote: > Ditto. > https://html.spec.whatwg.org/multipage/webappapis.html#incumbent-settings-object Done. https://codereview.chromium.org/1542623004/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:162: and N isolated worlds. An isolate world of a worker thread consists of On 2015/12/24 07:15:15, Yuki wrote: > s/An isolate world of/An isolate of/ Done.
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1542623004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1542623004/20001
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Failed to apply the patch.
Message was sent while issue was closed.
Description was changed from ========== Add documentation about Isolate, Context, World, Frames etc BUG= ========== to ========== Add documentation about Isolate, Context, World, Frames etc BUG= Committed: https://crrev.com/b6f387262b5241d6eb278604b59a8527e7996ee6 Cr-Commit-Position: refs/heads/master@{#366959} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/b6f387262b5241d6eb278604b59a8527e7996ee6 Cr-Commit-Position: refs/heads/master@{#366959}
Message was sent while issue was closed.
https://codereview.chromium.org/1542623004/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md (right): https://codereview.chromium.org/1542623004/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:11: An isolate is a concept of a thread in V8. that's not really true, it's just the way Blink uses V8 https://codereview.chromium.org/1542623004/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:68: the callee function is pushed onto the stack. When that function returns, that's not true in general (e.g. tail calls don't do that) https://codereview.chromium.org/1542623004/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:73: of the stack) a current context. unless the debugger is active, then the top context might also be the debugger context :-/ https://codereview.chromium.org/1542623004/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:111: context an entered context. any V8 C++ API that takes a context argument enters that context. https://codereview.chromium.org/1542623004/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:136: An isolate of a worker thread has 1 worker world and 0 isolated world. is that also true for compositor workers? https://codereview.chromium.org/1542623004/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:140: All worlds in one isolate share underlying C++ DOM objects, for one specific iframe that is https://codereview.chromium.org/1542623004/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:196: For compatibility reasons (although this is not speced), At least some specs explicitly state this
Message was sent while issue was closed.
Uploaded a CL to apply jochen's comments: https://codereview.chromium.org/1567103003/ https://codereview.chromium.org/1542623004/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md (right): https://codereview.chromium.org/1542623004/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:11: An isolate is a concept of a thread in V8. On 2016/01/07 10:18:44, jochen wrote: > that's not really true, it's just the way Blink uses V8 Reworded. https://codereview.chromium.org/1542623004/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:68: the callee function is pushed onto the stack. When that function returns, On 2016/01/07 10:18:45, jochen wrote: > that's not true in general (e.g. tail calls don't do that) I don't want to run into that kind of details since this document is written for developers who are not familiar with isolate, context, world etc... https://codereview.chromium.org/1542623004/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:73: of the stack) a current context. On 2016/01/07 10:18:44, jochen wrote: > unless the debugger is active, then the top context might also be the debugger > context :-/ Added the explanation for a debugger context. https://codereview.chromium.org/1542623004/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:111: context an entered context. On 2016/01/07 10:18:45, jochen wrote: > any V8 C++ API that takes a context argument enters that context. Added. https://codereview.chromium.org/1542623004/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:136: An isolate of a worker thread has 1 worker world and 0 isolated world. On 2016/01/07 10:18:44, jochen wrote: > is that also true for compositor workers? This description is true, but it is not true that one isolate maps to one compositor worker thread. Updated the explanation of the isolate. https://codereview.chromium.org/1542623004/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:140: All worlds in one isolate share underlying C++ DOM objects, On 2016/01/07 10:18:45, jochen wrote: > for one specific iframe that is What do you mean? https://codereview.chromium.org/1542623004/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/core/v8/V8BindingDesign.md:196: For compatibility reasons (although this is not speced), On 2016/01/07 10:18:45, jochen wrote: > At least some specs explicitly state this Removed the words in the parentheses. |
