First of all, would you help me understand why it's okay to create a location wrapper on the current context? Per the spec, the location wrapper needs to be created on the creation context (although I understand you don't have the creation context if it's a remote frame), right?

Note that wrapper->CreationContext() returns the context on which the wrapper gets created. If you wrongly use the current context, wrapper->CreationContext() will return a wrong context later.

Regarding the setHiddenValue, we're planning to replace it with the wrapper tracing.

The reason it's OK to use the current context as the creation context is because the cross-origin wrapper is never shared with any other contexts -- it's per-context and bound to the current context (e.g. the context that accessed the location property). It still goes through all the normal access checks, so attempting to get/set random properties that aren't cross-origin will still fail.

Re: wrapper->CreationContext() returning the current context -- I think we do want that behavior here, so it actually seems OK.

Re: wrapper tracing, how far away is that? It'll simplify the lifetime management a bit if I don't have to worry about it, but I'm pretty sure I need to maintain a cache either way, so crossOriginWindow.location === crossOriginWindow.location is true.

> Re: wrapper tracing, how far away is that?

Wrapper tracing is available but we don't have a bandwidth to rewrite setHiddenValue with it for now :/ (If that's needed, please go ahead :-)

I'm just curious but how many remote wrappers do you need to support? Only Location? If the main frame has 10 remote frames, do we need to keep 11 Location wrappers?

Re: wrapper tracing, it's not a blocker. Perhaps I can help with this after this is done.

The only object that needs a remote wrapper is Location. Unfortunately, this does mean that we would need to keep a Location wrapper per-context for each cross-origin access to the location object. I think it will still be quite a bit smaller than keeping a full v8::Context though.

Strictly speaking, we don't need to do this if the cross-origin window has a v8::Context, but the code will be easier to maintain if we have a consistent cross-origin path.

> Strictly speaking, we don't need to do this if the cross-origin window has a v8::Context, but the code will be easier to maintain if we have a consistent cross-origin path.

Just to clarify, we don't want to create a v8::Context on a cross-origin window because it wastes memory, right?

Would it be an option to store a location wrapper on RemoteFrame? And add a custom binding for the attribute getter of window.location so that it returns a proper location wrapper.

We'd still need to create the wrapper object in some context, wouldn't we? And that means we can't share the wrapper object, because the prototype chain can leak between contexts like in the http/tests/security/location-prototype.html  layout test.

However, if v8 has an API to create a object object instance (similar to FunctionTemplate::NewRemoteInstance), then that might work...

So what we need is something like:

  class V8PerContextData {
    HashSet<RemoteFrame*, ScopedPersistent<v8::Location>> m_setOfLocationWrappers;
  };

?

> However, if v8 has an API to create a object object instance (similar to FunctionTemplate::NewRemoteInstance), then that might work...

In this case, who maintains the mapping from RemoteFrame* to a location wrapper?

> So what we need is something like:
>
>  class V8PerContextData {
>    HashSet<RemoteFrame*, ScopedPersistent<v8::Location>> m_setOfLocationWrappers;
>  };
>
> ?

It would be something like:
  HeapHashMap<WeakMember<Location>, ScopedPersistent<v8::Object>> m_crossOriginLocationWrappers;

However, I'm not 100% sure if it's OK to put this on V8PerContextData. This is cleared when the window proxy is detached. However, JS closures can  retain references to a crossOriginWindow, and if the JS closure accesses crossOriginWindow.location, then we would prefer to return the cached wrapper...

> In this case, who maintains the mapping from RemoteFrame* to a location wrapper?

If we have some way of creating a remote object instance, I think we can just share it among all the cross-origin contexts. However, I'm not sure if we would be able to store it as a hidden value on the global proxy object itself, due to the access checks. So we would have to find an alternate storage location (perhaps it could be stored in Location itself; I'm not 100% sure if this is safe or if it would cause cycles).

> It would be something like:
>   HeapHashMap<WeakMember<Location>, ScopedPersistent<v8::Object>>  m_crossOriginLocationWrappers;
>
> However, I'm not 100% sure if it's OK to put this on V8PerContextData. This is cleared when the window proxy is detached. However, JS closures can  retain references to a crossOriginWindow, and if the JS closure accesses crossOriginWindow.location, then we would prefer to return the cached wrapper...

The real problem here is that we're disposing V8PerContextData when the window proxy gets detached. This stops many things from working on a detached frame.

Previously we had to dispose V8PerContextData to break a reference cycle between Blink and V8, but once the wrapper tracing is available, I think we can work around the cycle problem. So we should try to stop disposing V8PerContextData anyway. Then both your problem and existing problems will be fixed :)

Please ignore this idea if it's too crazy or inefficient, but my first idea was to re-use the existing trick that we're currently using for fooOriginSafeMethodGetter.

v8::FunctionTemplate::GetFunction returns the unique function instance per v8::Context.  This is useful for cross-origin representation of Location, I think.

  v8::Context currentContext = ...;
  v8::FunctionTemplate locationTemplate = ...;
  v8::Function location = locationTemplate->GetFunction(currentContext);

Then, we get |location| object unique per v8::Context as a cross-origin representation.  V8 GC may collect this object if author script doesn't hold it.  But, anyway, we can get the unique instance per context any time.  Blink doesn't need to control the mappings nor lifetime in V8PerContextData.

If we don't like that |location| is of type v8::Function, we could do the following.

  v8::FunctionTemplate template = ...;
  v8::Function interface = template->GetFunction(currentContext);
  if (interface does NOT have the private property) {
    v8::Object location = interface->New(currentContext);
    interface->SetPrivateProperty(location);
    location->SetPrivateProperty(interface);
  }
  return the private property (= location);

As long as the location object alive, the interface object is alive, too, and vice versa.  So, the magic of v8::FT::GetFunction still works.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9ead0def6f12d7152260f2eb21e78c7e09cd47ee

commit 9ead0def6f12d7152260f2eb21e78c7e09cd47ee
Author: dcheng <dcheng@chromium.org>
Date: Fri Jan 27 10:31:07 2017

Use NewRemoteInstance() to create cross-origin objects.

Also use the current context as the creation context when
returning the cross-origin named enumerator's results.

BUG= 682822 

Review-Url: https://codereview.chromium.org/2640123006
Cr-Commit-Position: refs/heads/master@{#446635}

[modify] https://crrev.com/9ead0def6f12d7152260f2eb21e78c7e09cd47ee/third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp
[modify] https://crrev.com/9ead0def6f12d7152260f2eb21e78c7e09cd47ee/third_party/WebKit/Source/bindings/scripts/v8_attributes.py
[modify] https://crrev.com/9ead0def6f12d7152260f2eb21e78c7e09cd47ee/third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl
[modify] https://crrev.com/9ead0def6f12d7152260f2eb21e78c7e09cd47ee/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceCheckSecurity.cpp
[modify] https://crrev.com/9ead0def6f12d7152260f2eb21e78c7e09cd47ee/third_party/WebKit/Source/core/frame/Window.idl