Entry realm issues

58 views
Skip to first unread message

Jens Widell

unread,
Mar 21, 2017, 10:46:06 AM3/21/17
to platform-architecture-dev
While working on Blink's implementation of the [PutForwards] WebIDL
feature, we discovered an issue with how the entry realm is tracked.
Blink currently uses v8::Isolate::GetEnteredOrMicrotaskContext() for
this, which does not always yield a correct result.

All suggestion would be very welcome.

Sorry about the long text.

Details
-------
The problem is that V8 enters a context whenever its C++ API is
called. This is usually good enough for tracking the entry realm
(although not exactly how it is described in the specification) but
also fails in some cases. Notably, when the Blink bindings layer is
called from V8, and calls back to V8 to convert values (e.g.
v8::Value::ToString()) or to access an object's properties (e.g.
v8::Object::Get()). V8 will enter whatever context Blink passes in to
that operation, but in this situation the entry realm should clearly
not change.

This error is currently observable in a somewhat contrived case such as

function f() {
document.title = {
toString() {
location.href = "foo.html";
}
};
}

where if f() is called from a function defined in another document,
the wrong URL would be used to resolve "foo.html" in the assignment.
With a correct [PutForwards] implementation, simply assigning
window.location or document.location (which then forwards to
location.href using v8::Object::Set()) will exhibit the same issue.

Possible solutions
------------------
1) Implement an entry realm stack in Blink, instead of relying on V8
to track this. I kind of like this approach, but for microtasks (e.g.
promises) we probably have to rely on V8, and so this approach
probably doesn't work in practice.

Here's an implementation of this: https://codereview.chromium.org/2764953002

(It doesn't work for at least promises.)

2) Adjust the V8 API so that a context doesn't get entered automatically. Either

a) not at all (forcing all clients to use v8::Context::Enter()), or
b) not for some operations (ToString()/Get()/Set()), or
c) not unless there's no currently entered context, or
c) make it optional for all/most operations.

I'm thinking 2(a) would be at least an inconvenient approach for many
embedders, including Blink. 2(b) feels rather arbitrary. 2(c) would
seem fine by me, but I don't really know what the consequences would
be. (And Blink would probably need to explicitly enter contexts in
some cases then.)

2(d) is kind of available already for at least Get()/Set(), by passing
in an empty context handle. But that feels like a hack.

Some links
----------
Entry realm spec: https://html.spec.whatwg.org/#entry
[PutForwards] CL: https://codereview.chromium.org/2733763003/

Jochen Eisinger

unread,
Mar 21, 2017, 10:55:52 AM3/21/17
to Jens Widell, platform-architecture-dev, verw...@chromium.org

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To post to this group, send email to platform-arc...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAEef6WwjW4pH6XR-%2Bvzg%2Bp4CVMcv3SRK_NMVovwspUks443bhw%40mail.gmail.com.

Jochen Eisinger

unread,
Mar 21, 2017, 11:03:13 AM3/21/17
to Jens Widell, platform-architecture-dev, verw...@chromium.org
btw, I only skimmed the spec, however, it's not entirely clear to me that blink isn't supposed to prepare to run script to call ToString() (and therefore enter the context)

Jens Widell

unread,
Mar 21, 2017, 11:27:23 AM3/21/17
to Jochen Eisinger, platform-architecture-dev, verw...@chromium.org
On Tue, Mar 21, 2017 at 4:02 PM, Jochen Eisinger <joc...@chromium.org> wrote:
> btw, I only skimmed the spec, however, it's not entirely clear to me that
> blink isn't supposed to prepare to run script to call ToString() (and
> therefore enter the context)

WebIDL does reference HTML's "prepare to run script" in some cases,
such as when invoking a callback function, but it doesn't e.g. in its
description of how to convert an ECMAScript value to a DOMString:
https://heycam.github.io/webidl/#es-DOMString

--
Jens

Kentaro Hara

unread,
Mar 22, 2017, 1:21:33 AM3/22/17
to Jens Widell, Jochen Eisinger, platform-architecture-dev, verw...@chromium.org
My 2 cents: I was thinking that ToString() should NOT "prepare to run script" because it is expected to run on a context that has been already entered.



2) Adjust the V8 API so that a context doesn't get entered automatically. Either
  a) not at all (forcing all clients to use v8::Context::Enter()), or
  b) not for some operations (ToString()/Get()/Set()), or
  c) not unless there's no currently entered context, or
  d) make it optional for all/most operations. 
 
I'm thinking 2(a) would be at least an inconvenient approach for many
embedders, including Blink. 2(b) feels rather arbitrary. 2(c) would
seem fine by me, but I don't really know what the consequences would
be. (And Blink would probably need to explicitly enter contexts in
some cases then.)

How inconvenient will 2(a) be? I guess that in common cases V8 APIs should just run on a context that has already been entered (and thus the client doesn't need to enter a context before calling the V8 APIs).


--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAEef6WyWoEsx4AgZrGUM-oaGZepnkY60iOsuy%3D2EF0A-AsmV9w%40mail.gmail.com.



--
Kentaro Hara, Tokyo, Japan

Jochen Eisinger

unread,
Mar 22, 2017, 3:33:48 AM3/22/17
to Kentaro Hara, Jens Widell, platform-architecture-dev, verw...@chromium.org

V8 could easily switch to not marking the context used in API functions as entered. We'd probably have to audit for internal uses of Context::Scope but I believe that's not a lot either.

How would you ensure that blink correctly enters entry contexts?

I guess we should also consider requiring an access check when invoking an api function with a context that's different from the entered context.


To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To post to this group, send email to platform-arc...@chromium.org.

Yuki Shiino

unread,
Mar 22, 2017, 3:50:18 AM3/22/17
to Jochen Eisinger, Kentaro Hara, Jens Widell, platform-architecture-dev, verw...@chromium.org
2017-03-22 16:33 GMT+09:00 Jochen Eisinger <joc...@chromium.org>:

V8 could easily switch to not marking the context used in API functions as entered. We'd probably have to audit for internal uses of Context::Scope but I believe that's not a lot either.

How would you ensure that blink correctly enters entry contexts?

For non-nested cases, could V8 check if the current context is empty or not?  If it's empty, Blink must forget to enters an context.

For nested cases, I don't have a good idea at the moment.

For promise's cases, the entry realm should be a creation context of (user-defined) fulfill or reject functions, and currently there seems no way for Blink to specify that realm.  IIUC, Blink calls v8::Promise::Resolver::{Resolve,Reject} and v8::Promise::Resolver::{Resolve,Reject} invokes (user-defined) fulfill or reject function respectively.  However, Blink has no access to a fulfill or reject function, so there seems no way to specify the entry realm.

I guess we should also consider requiring an access check when invoking an api function with a context that's different from the entered context.

Do you mean that the first v8::Context argument must always be the current context?

 

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CALjhuif6nQjhabi5r3AL6v-i%2B17N3e-JEdFKWu%2Bf-OzJ0b-XaA%40mail.gmail.com.

Yuki Shiino

unread,
Mar 22, 2017, 3:54:09 AM3/22/17
to Yuki Shiino, Jochen Eisinger, Kentaro Hara, Jens Widell, platform-architecture-dev, verw...@chromium.org
By the way, the Incumbent realm has the same problem as the Entry realm has, so this will help Blink implement the Incumbent realm, too.


To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.

To post to this group, send email to platform-architecture-dev@chromium.org.

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.

To post to this group, send email to platform-architecture-dev@chromium.org.

Jochen Eisinger

unread,
Mar 22, 2017, 4:23:36 AM3/22/17
to Yuki Shiino, Kentaro Hara, Jens Widell, platform-architecture-dev, verw...@chromium.org
On Wed, Mar 22, 2017 at 8:54 AM Yuki Shiino <yukis...@chromium.org> wrote:
By the way, the Incumbent realm has the same problem as the Entry realm has, so this will help Blink implement the Incumbent realm, too.


2017-03-22 16:49 GMT+09:00 Yuki Shiino <yukis...@chromium.org>:


2017-03-22 16:33 GMT+09:00 Jochen Eisinger <joc...@chromium.org>:

V8 could easily switch to not marking the context used in API functions as entered. We'd probably have to audit for internal uses of Context::Scope but I believe that's not a lot either.

How would you ensure that blink correctly enters entry contexts?

For non-nested cases, could V8 check if the current context is empty or not?  If it's empty, Blink must forget to enters an context.

V8 would still push the context on the context stack, so we'd never have an empty context. We'd just not mark it as entered.
 

For nested cases, I don't have a good idea at the moment.

For promise's cases, the entry realm should be a creation context of (user-defined) fulfill or reject functions, and currently there seems no way for Blink to specify that realm.  IIUC, Blink calls

Currently, the entry realm is the creation context of the promise constructor
 
v8::Promise::Resolver::{Resolve,Reject} and v8::Promise::Resolver::{Resolve,Reject} invokes (user-defined) fulfill or reject function respectively.  However, Blink has no access to a fulfill or reject function, so there seems no way to specify the entry realm.

I guess we should also consider requiring an access check when invoking an api function with a context that's different from the entered context.

Do you mean that the first v8::Context argument must always be the current context?


No, it just needs to be accessible to the entered context.
 
 

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To post to this group, send email to platform-arc...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To post to this group, send email to platform-arc...@chromium.org.

Jens Widell

unread,
Mar 22, 2017, 4:46:22 AM3/22/17
to Jochen Eisinger, Kentaro Hara, platform-architecture-dev, verw...@chromium.org
On Wed, Mar 22, 2017 at 8:33 AM, Jochen Eisinger <joc...@chromium.org> wrote:
> V8 could easily switch to not marking the context used in API functions as
> entered. We'd probably have to audit for internal uses of Context::Scope but
> I believe that's not a lot either.
>
> How would you ensure that blink correctly enters entry contexts?

That would be the tricky bit. I believe all v8::Function::Call() and
v8::Script::Run() in Blink (except for a few unit tests) go via
V8ScriptRunner already, so ensuring we enter a context for those is
easy. But e.g. property accesses are spread out a lot more in the
code, so keeping track of whether there's an entered context at all of
those call sites would be tricky.

On the other hand, there is no support in the specification for
entering a context whenever we feel like it, so we could also use an
approach where we don't try to make sure always to enter a context
(i.e. only do it when the specification says to), and then deal with
the possibility of not having an entered context in the few places
where we actually use it.


> I guess we should also consider requiring an access check when invoking an
> api function with a context that's different from the entered context.

The most common case I guess would be that Blink calls V8 back with
V8's current context (which may be different from the entered
context.) In that case an access check ought to be unnecessary, since
that context is already the current one.

If Blink calls V8 with a context that is neither the entered nor the
current, an access check seems reasonable to me. But I can't point to
where any specification supports having one.

--
Jens
> https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CALjhuif6nQjhabi5r3AL6v-i%2B17N3e-JEdFKWu%2Bf-OzJ0b-XaA%40mail.gmail.com.

Jens Widell

unread,
Mar 29, 2017, 8:14:27 AM3/29/17
to Jochen Eisinger, Kentaro Hara, platform-architecture-dev, verw...@chromium.org
Hi all,

I've been experimenting and come up with an approach that seems
acceptable to me, and that seems to work (at least according to
LayoutTests/). Here's the fairly small changes:

1) Modify v8's API to only set a context as current, and not entering
it automatically (https://codereview.chromium.org/2782613003), and

2) enter a context in V8ScriptRunner in Blink, where that wasn't
clearly already done.

With these changes, GetEnteredOrMicrotaskContext(), as it's used
today, produces the correct result.

WDYT?

The involved code in Blink might do well with some restructuring as
part of this; it currently enters contexts in varying and sometimes
unclear ways (e.g. using ScriptState::Scope, or ScopedFrameBlamer),
and then calls V8 with isolate->GetCurrentContext() even though that
ought to be a context that was just entered.

I've tested these changes by running all LayoutTests/ with a
CHECK(!GetEnteredOrMicrotaskContext().IsEmpty()) added to all
generated bindings methods, constructors, getters and setters, and
this failed in a small number of cases, all related to console.log()
and related accessing of properties by V8. Probably easy to fix, and
also probably a situation where one could argue there is no entry
realm, really.

--
Jens

Yuki Shiino

unread,
Mar 29, 2017, 8:53:14 AM3/29/17
to Jens Widell, Jochen Eisinger, Kentaro Hara, platform-architecture-dev, verw...@chromium.org
I support this approach.  I think that this is the simplest way.

The issue about the Entry realm for Promise remains as is, but it should be handled separately.  Jens' approach doesn't affect Promise's case, I assume.
Details: When a promise is resolved, a fulfill/reject function is invoked with setting the Entry realm to the creation context of the promise object, and this is done by V8.  I suppose that Jens' patch doesn't change this behavior.
is discussing that we should set the Entry realm to the fulfill/reject function's context instead of the promise's context.  This is my understanding.

Cheers,
Yuki Shiino



>>>> To post to this group, send email to

>>>>
>>>> To view this discussion on the web visit
>>>> https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAEef6WyWoEsx4AgZrGUM-oaGZepnkY60iOsuy%3D2EF0A-AsmV9w%40mail.gmail.com.
>>>
>>>
>>>
>>>
>>> --
>>> Kentaro Hara, Tokyo, Japan
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "platform-architecture-dev" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAEef6WyXL91tYi5fUGHk4jOCEuSjxDwWL1hdrJhVHuoMRpj0eQ%40mail.gmail.com.

Jens Widell

unread,
Mar 29, 2017, 9:00:56 AM3/29/17
to Yuki Shiino, Jochen Eisinger, Kentaro Hara, platform-architecture-dev, verw...@chromium.org
On Wed, Mar 29, 2017 at 2:52 PM, Yuki Shiino <yukis...@chromium.org> wrote:
> I support this approach. I think that this is the simplest way.
>
> The issue about the Entry realm for Promise remains as is, but it should be
> handled separately. Jens' approach doesn't affect Promise's case, I assume.
> Details: When a promise is resolved, a fulfill/reject function is invoked
> with setting the Entry realm to the creation context of the promise object,
> and this is done by V8. I suppose that Jens' patch doesn't change this
> behavior.

Right, my proposed patches would not change anything relating to
promises. The entry realm for promise resolution/rejection is defined
by GetMicrotaskContext(), as GetEnteredContext() will be empty, and
GetEnteredOrMicrotaskContext() then returns the microtask context.
With my patches we're less likely to "promote" the microtask context
to an entered context, but this would not affect what we use as entry
realm in the end.

--
Jens


> https://github.com/whatwg/html/pull/1189#issuecomment-224950188
> is discussing that we should set the Entry realm to the fulfill/reject
> function's context instead of the promise's context. This is my
> understanding.
>
>> >>>> email to platform-architect...@chromium.org.
>> >>>> To post to this group, send email to
>> >>>> platform-arc...@chromium.org.
>> >>>>
>> >>>> To view this discussion on the web visit
>> >>>>
>> >>>> https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAEef6WyWoEsx4AgZrGUM-oaGZepnkY60iOsuy%3D2EF0A-AsmV9w%40mail.gmail.com.
>> >>>
>> >>>
>> >>>
>> >>>
>> >>> --
>> >>> Kentaro Hara, Tokyo, Japan
>> >>
>> >> --
>> >> You received this message because you are subscribed to the Google
>> >> Groups
>> >> "platform-architecture-dev" group.
>> >> To unsubscribe from this group and stop receiving emails from it, send
>> >> an
>> >> email to platform-architect...@chromium.org.
>> >> To post to this group, send email to
>> >> platform-arc...@chromium.org.
>> >> To view this discussion on the web visit
>> >>
>> >> https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CALjhuif6nQjhabi5r3AL6v-i%2B17N3e-JEdFKWu%2Bf-OzJ0b-XaA%40mail.gmail.com.
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "platform-architecture-dev" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to platform-architect...@chromium.org.
>> To post to this group, send email to
>> platform-arc...@chromium.org.
>> To view this discussion on the web visit
>> https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAEef6WyXL91tYi5fUGHk4jOCEuSjxDwWL1hdrJhVHuoMRpj0eQ%40mail.gmail.com.
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "platform-architecture-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to platform-architect...@chromium.org.
> To post to this group, send email to platform-arc...@chromium.org.
> To view this discussion on the web visit
> https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAN0uC_Qpk7Hftx938MFR99BJoWFuYUuZGQCm7xJFpSQexthmDg%40mail.gmail.com.

Kentaro Hara

unread,
Mar 29, 2017, 9:40:30 AM3/29/17
to Jens Widell, Yuki Shiino, Jochen Eisinger, platform-architecture-dev, verw...@chromium.org
1) Modify v8's API to only set a context as current, and not entering
it automatically (https://codereview.chromium.org/2782613003), and 
 
2) enter a context in V8ScriptRunner in Blink, where that wasn't
clearly already done. 
 
With these changes, GetEnteredOrMicrotaskContext(), as it's used
today, produces the correct result. 
 
WDYT? 
 
The involved code in Blink might do well with some restructuring as
part of this; it currently enters contexts in varying and sometimes
unclear ways (e.g. using ScriptState::Scope, or ScopedFrameBlamer),
and then calls V8 with isolate->GetCurrentContext() even though that
ought to be a context that was just entered.

V8ScriptRunner is designed with an assumption that the caller site has already entered a correct context (using ScriptState::Scope etc). I designed it that way because in order to call a method of V8ScriptRunner, you need to prepare a bunch of things before calling it, where you already need to enter that context. In other words, things should happen in this order: 1) Enter a context (using ScriptState::Scope), 2) Prepare a bunch of V8 things, 3) Call a method of V8ScriptRunner. That's why V8ScriptRunner doesn't explicitly enter that context.

I'm fine with changing the design but want to understand one thing:

The fact that entering a context in V8ScriptRunner fixed your problem would indicate that there're call sites that are not entering that context before calling V8ScriptRunner. What are the call sites specifically? Would it be an option to fix the call sites so that they enter that context before calling V8ScriptRunner?




>> >>>> To post to this group, send email to

>> >>>>
>> >>>> To view this discussion on the web visit
>> >>>>
>> >>>> https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAEef6WyWoEsx4AgZrGUM-oaGZepnkY60iOsuy%3D2EF0A-AsmV9w%40mail.gmail.com.
>> >>>
>> >>>
>> >>>
>> >>>
>> >>> --
>> >>> Kentaro Hara, Tokyo, Japan
>> >>
>> >> --
>> >> You received this message because you are subscribed to the Google
>> >> Groups
>> >> "platform-architecture-dev" group.
>> >> To unsubscribe from this group and stop receiving emails from it, send
>> >> an

>> >> To post to this group, send email to

>> >> To view this discussion on the web visit
>> >>
>> >> https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CALjhuif6nQjhabi5r3AL6v-i%2B17N3e-JEdFKWu%2Bf-OzJ0b-XaA%40mail.gmail.com.
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "platform-architecture-dev" group.
>> To unsubscribe from this group and stop receiving emails from it, send an

>> To post to this group, send email to

>> To view this discussion on the web visit
>> https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAEef6WyXL91tYi5fUGHk4jOCEuSjxDwWL1hdrJhVHuoMRpj0eQ%40mail.gmail.com.
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "platform-architecture-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an

Jens Widell

unread,
Mar 29, 2017, 9:49:20 AM3/29/17
to Kentaro Hara, Yuki Shiino, Jochen Eisinger, platform-architecture-dev, verw...@chromium.org
On Wed, Mar 29, 2017 at 3:39 PM, Kentaro Hara <har...@chromium.org> wrote:

> V8ScriptRunner is designed with an assumption that the caller site has
> already entered a correct context (using ScriptState::Scope etc). I designed
> it that way because in order to call a method of V8ScriptRunner, you need to
> prepare a bunch of things before calling it, where you already need to enter
> that context. In other words, things should happen in this order: 1) Enter a
> context (using ScriptState::Scope), 2) Prepare a bunch of V8 things, 3) Call
> a method of V8ScriptRunner. That's why V8ScriptRunner doesn't explicitly
> enter that context.

Ah. In that case it may not be necessary to modify V8ScriptRunner. I
did not really investigate whether V8ScriptRunner always had an
entered context when called. I did however observe that it sometimes
was called from a function that did enter a context, and that it also
sometimes enters one itself already, using ScopedFrameBlamer.

I will check that part some more. It may be that we don't need changes
in that area.


> I'm fine with changing the design but want to understand one thing:
>
> The fact that entering a context in V8ScriptRunner fixed your problem would
> indicate that there're call sites that are not entering that context before
> calling V8ScriptRunner. What are the call sites specifically? Would it be an
> option to fix the call sites so that they enter that context before calling
> V8ScriptRunner?

So, to clarify: I did not actually find (or look for) any instance
where a context obviously wasn't entered already. The situation I was
confronted with was

<unknown> -> V8ScriptRunner -> V8

and I removed context entering from V8, so I assumed adding one to
V8ScriptRunner instead was potentially necessary. And if it wasn't
necessary, it would not hurt to add it, since the addition simply
represented moving it one step down in the callstack.

Fixing the call sites (if necessary) would certainly be a possibility.
By the sounds of thing, also the right thing to do, if so. I will
investigate.
>> >> >>>> email to platform-architect...@chromium.org.
>> >> >>>> To post to this group, send email to
>> >> >>>> platform-arc...@chromium.org.
>> >> >>>>
>> >> >>>> To view this discussion on the web visit
>> >> >>>>
>> >> >>>>
>> >> >>>> https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAEef6WyWoEsx4AgZrGUM-oaGZepnkY60iOsuy%3D2EF0A-AsmV9w%40mail.gmail.com.
>> >> >>>
>> >> >>>
>> >> >>>
>> >> >>>
>> >> >>> --
>> >> >>> Kentaro Hara, Tokyo, Japan
>> >> >>
>> >> >> --
>> >> >> You received this message because you are subscribed to the Google
>> >> >> Groups
>> >> >> "platform-architecture-dev" group.
>> >> >> To unsubscribe from this group and stop receiving emails from it,
>> >> >> send
>> >> >> an
>> >> >> email to platform-architect...@chromium.org.
>> >> >> To post to this group, send email to
>> >> >> platform-arc...@chromium.org.
>> >> >> To view this discussion on the web visit
>> >> >>
>> >> >>
>> >> >> https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CALjhuif6nQjhabi5r3AL6v-i%2B17N3e-JEdFKWu%2Bf-OzJ0b-XaA%40mail.gmail.com.
>> >>
>> >> --
>> >> You received this message because you are subscribed to the Google
>> >> Groups
>> >> "platform-architecture-dev" group.
>> >> To unsubscribe from this group and stop receiving emails from it, send
>> >> an
>> >> email to platform-architect...@chromium.org.
>> >> To post to this group, send email to
>> >> platform-arc...@chromium.org.
>> >> To view this discussion on the web visit
>> >>
>> >> https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAEef6WyXL91tYi5fUGHk4jOCEuSjxDwWL1hdrJhVHuoMRpj0eQ%40mail.gmail.com.
>> >
>> >
>> > --
>> > You received this message because you are subscribed to the Google
>> > Groups
>> > "platform-architecture-dev" group.
>> > To unsubscribe from this group and stop receiving emails from it, send
>> > an
>> > email to platform-architect...@chromium.org.
>> > To post to this group, send email to
>> > platform-arc...@chromium.org.
>> > To view this discussion on the web visit
>> >
>> > https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAN0uC_Qpk7Hftx938MFR99BJoWFuYUuZGQCm7xJFpSQexthmDg%40mail.gmail.com.
>
>
>
>
> --
> Kentaro Hara, Tokyo, Japan
>
> --
> You received this message because you are subscribed to the Google Groups
> "platform-architecture-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to platform-architect...@chromium.org.
> To post to this group, send email to platform-arc...@chromium.org.
> To view this discussion on the web visit
> https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABg10jz-_TKi2f-a3VskQpLsk%3DRQaQPk876an194tWQtsMABJA%40mail.gmail.com.

Jochen Eisinger

unread,
Mar 29, 2017, 9:54:52 AM3/29/17
to Jens Widell, Kentaro Hara, Yuki Shiino, platform-architecture-dev, verw...@chromium.org
in addition to the V8 change, we'll probably also have to audit those callsites: https://cs.chromium.org/search/?q=v8::Context::Scope+package:%5Echromium$+file:src/v8/src+-file:d8&type=cs

Kentaro Hara

unread,
Mar 29, 2017, 9:55:40 AM3/29/17
to Jens Widell, Yuki Shiino, Jochen Eisinger, platform-architecture-dev, verw...@chromium.org
So, to clarify: I did not actually find (or look for) any instance
where a context obviously wasn't entered already. The situation I was
confronted with was 
 
  <unknown> -> V8ScriptRunner -> V8 
 
and I removed context entering from V8, so I assumed adding one to
V8ScriptRunner instead was potentially necessary. And if it wasn't
necessary, it would not hurt to add it, since the addition simply
represented moving it one step down in the callstack. 

Ah, understood.

Then I'd suggest fixing the call sites instead of V8ScriptRunner (if necessary) because V8ScriptRunner is not the only way to enter V8. Get, Set, ToString etc can also enter V8 and execute any arbitrary script. In that case, we anyway need to enter a correct context before calling those V8 APIs.

i.e., it's important that you're in a correct context whenever you're calling V8 APIs (not limited to V8ScriptRunner).




>> >> >>>> To post to this group, send email to

>> >> >>>>
>> >> >>>> To view this discussion on the web visit
>> >> >>>>
>> >> >>>>
>> >> >>>> https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAEef6WyWoEsx4AgZrGUM-oaGZepnkY60iOsuy%3D2EF0A-AsmV9w%40mail.gmail.com.
>> >> >>>
>> >> >>>
>> >> >>>
>> >> >>>
>> >> >>> --
>> >> >>> Kentaro Hara, Tokyo, Japan
>> >> >>
>> >> >> --
>> >> >> You received this message because you are subscribed to the Google
>> >> >> Groups
>> >> >> "platform-architecture-dev" group.
>> >> >> To unsubscribe from this group and stop receiving emails from it,
>> >> >> send
>> >> >> an

>> >> >> To post to this group, send email to

>> >> >> To view this discussion on the web visit
>> >> >>
>> >> >>
>> >> >> https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CALjhuif6nQjhabi5r3AL6v-i%2B17N3e-JEdFKWu%2Bf-OzJ0b-XaA%40mail.gmail.com.
>> >>
>> >> --
>> >> You received this message because you are subscribed to the Google
>> >> Groups
>> >> "platform-architecture-dev" group.
>> >> To unsubscribe from this group and stop receiving emails from it, send
>> >> an

>> >> To post to this group, send email to

>> >> To view this discussion on the web visit
>> >>
>> >> https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAEef6WyXL91tYi5fUGHk4jOCEuSjxDwWL1hdrJhVHuoMRpj0eQ%40mail.gmail.com.
>> >
>> >
>> > --
>> > You received this message because you are subscribed to the Google
>> > Groups
>> > "platform-architecture-dev" group.
>> > To unsubscribe from this group and stop receiving emails from it, send
>> > an

>> > To post to this group, send email to

>> > To view this discussion on the web visit
>> >
>> > https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAN0uC_Qpk7Hftx938MFR99BJoWFuYUuZGQCm7xJFpSQexthmDg%40mail.gmail.com.
>
>
>
>
> --
> Kentaro Hara, Tokyo, Japan
>
> --
> You received this message because you are subscribed to the Google Groups
> "platform-architecture-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an

> To view this discussion on the web visit
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAEef6WwYc5SvsHL0DeG6NV0DzEVtND6ccMtVZ_DjhQDRfRK6aA%40mail.gmail.com.

Jens Widell

unread,
Apr 21, 2017, 6:42:39 AM4/21/17
to Jochen Eisinger, Kentaro Hara, platform-architecture-dev, verw...@chromium.org
On Wed, Mar 29, 2017 at 2:14 PM, Jens Widell <j...@opera.com> wrote:
> Hi all,
>
> I've been experimenting and come up with an approach that seems
> acceptable to me, and that seems to work (at least according to
> LayoutTests/). Here's the fairly small changes:
>
> 1) Modify v8's API to only set a context as current, and not entering
> it automatically (https://codereview.chromium.org/2782613003), and
>
> 2) enter a context in V8ScriptRunner in Blink, where that wasn't
> clearly already done.
>
> With these changes, GetEnteredOrMicrotaskContext(), as it's used
> today, produces the correct result.
>
> WDYT?

Reviving thread. Sorry that it's taken a while to get here.

Design doc: https://docs.google.com/document/d/1VwlOQhfjAabPC4sn-yJ01iprGCRo7YWY-9TamrQKMuA/edit?usp=sharing

Nothing much new there; the proposal is simply that V8 stops entering
contexts implicitly.

To discuss: V8 inspector sometimes triggers execution without entering
a context, which can at least in some cases end up calling into Blink.
Should our strategy be that a context must always be entered, and fix
V8 inspector, or should we alter Blink to reject e.g.
document.open()/location.assign()/et.c. calls made from "odd" call
sites like V8 inspector, where no specification really covers what
should be the entered context?

--
Jens

Daniel Cheng

unread,
Apr 21, 2017, 6:49:41 AM4/21/17
to Jens Widell, Jochen Eisinger, Kentaro Hara, platform-architecture-dev, verw...@chromium.org
I think it would make sense to fix the inspector; while no specification really defines what the entered context should be, in practice, I think it should be the context of the frame that's being inspected, right?

Daniel
 

--
Jens

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To post to this group, send email to platform-arc...@chromium.org.

Kentaro Hara

unread,
Apr 21, 2017, 9:06:52 AM4/21/17
to Daniel Cheng, Jens Widell, Jochen Eisinger, platform-architecture-dev, verw...@chromium.org
+1

Also the proposed design in the doc sounds reasonable to me.



Daniel
 

--
Jens

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.

Yuki Shiino

unread,
Apr 26, 2017, 5:23:51 AM4/26/17
to Kentaro Hara, Daniel Cheng, Jens Widell, Jochen Eisinger, platform-architecture-dev, verw...@chromium.org
Sorry for the late response.

+1 for "the context of the frame that's being inspected".
It's most intuitive and reasonable, I think.

Cheers,
Yuki Shiino


To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.

To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABg10jyUikKQ1-3vMJouMzpx1qLdNtYnqB%2BBhJ%3DmkaDeRM3ELg%40mail.gmail.com.

Reply all
Reply to author
Forward
0 new messages