Bug 31317 - Implement SharedScript API.
Summary: Implement SharedScript API.
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Dmitry Titov
URL:
Keywords:
Depends on: 31427 31444 31569 31857
Blocks:
  Show dependency treegraph
 
Reported: 2009-11-10 14:20 PST by Dmitry Titov
Modified: 2009-12-07 16:59 PST (History)
10 users (show)

See Also:


Attachments
Prototype Implementation - not for review. (103.80 KB, patch)
2009-11-10 17:02 PST, Dmitry Titov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Titov 2009-11-10 14:20:14 PST
Spec so far: http://docs.google.com/View?docID=0ATms0L1uQbSdZHh2M2N0aF8ycm5qOXdmZno

The spec reflects the feedback received during discussions in WhatWG, Chrome and webkit-dev lists. We have a strong request from Google app developers for this functionality so the plan is to move forward and implement it as an experimental WebKit API. Some details of the implementation plan (similar to Notifications):
- have API prefixed, like WebKitGlobalScript to make sure it doesn't clash with future API
- under #if ENABLE(GLOBAL_SCRIPT), additional runtime flag in V8
- implemented for both JSC and V8. I think most (if not all) code for the feature is inside WebCore, so it doesn't need embedder's support.
- as it is experimental, it can be changed or removed over time.

The implementation direction:
- have WebKitGlobalScript.idl + GlobalScriptContext.idl
- the GlobalScriptContext is derived from ScriptExecutionContext, there is also GlobalScriptController to run JS in a document-less context but not on a separate thread as Workers do.
- start with almost-empty GlobalScriptContext and add timers, XHR, Database in the process.
- enable layout tests as soon as possible (for ports which will enable the feature)
- fix issues stemming form mixing lexical and dynamic GlobalObject as they are found
- figure out integration with Inspector (later)

I'll attach a 'prototype' patch soon - not for review, just for possible feedback on a direction. The actual implementation will likely come in a several smaller patches.
Comment 1 Dmitry Titov 2009-11-10 17:02:12 PST
Created attachment 42910 [details]
Prototype Implementation - not for review.

The first cut on implementation - not for review, but it is working and shows the direction, so feedback is highly welcome.
JSC bindings only for now.
Comment 2 Geoffrey Garen 2009-11-20 13:48:45 PST
Hi Dmitry.

I started to look over your patch, with a focus on the JSC bindings, but then I realized that I really needed to look over your specification first. So, here's a set of comments about the specification:

> GlobalScript - shared, same-thread JS context

I think the word "global" is overused in JavaScript, and often misleading.

"window" is described as the "global" object, so you're creating the concept of a
global global object, which encompasses a set of sub-objects which are themselves
global (but not global global!).

Also, the thing you're calling "global" is not actually global, since there can be
more than one of it.

This is weird. Let's not add to the pile.

How about the name SharedScript instead?

In addition to avoiding the "global" pitfall, I think "Shared" does more to
communicate the interesting quirk that many clients can load the same SharedScript,
but the script will actually load only once, and all clients will share it.

> Currently there is no mechanism to directly share DOM, code and data on the
> same ui thread across several pages of the web application.

This statement isn't really true, and it made it hard for me to understand your
proposal initially.

You can share DOM, code, and data on the same UI thread across several pages
using a window object.

I think what you're getting at here is that there's not way to do what you want
without opening and/or persisting a user-visible window.

[ Also, notice your own use of the word "share" here! :) ]

Proposal

> All pages connected to the same Global Script should run on the same thread, 
> in the same process (they logically form an 'application instance').  Since 
> this is not always technically possible, it should be legal (and not break the 
> applications) for there to be duplicate global script contexts within a UA.

I don't understand this. Wouldn't duplicate global script contexts break the
feature of sharing DOM and data?

For example, if I stored a session token in a global script context, wouldn't a
page with a duplicate global script context be forced to log in a second time,
and then end up in a different session anyway, possible breaking the app?

> The property "context" on the GlobalScript returns the global scope object of 
> the loaded and initialized script. 

Notice how awkward it is to describe a "global" object returning a "global" object,
with the two "globals" being completely different.

> opening a tearoff page for the chat with 'username' (this code lives in 
> globalScript so it can easily check for 'app-wide' info like whether or not the
> chat window for 'username' was already open:

Notice that this example is not true if you allow for duplicate global script
contexts, since one global script context might know if a chat window for
'username' were already open, while another did not.

> Another goodness is that observed behavior does not depend on whether the
> script is already loaded or not.

Note that observed behavior of the load and error events does depend on whether
the script is already loaded or not, since they fire synchronously if the script
is already loaded, and asynchronously otherwise. These two behaviors are
philosophically inconsistent.

> It is not necessary to have this behavior deterministic or explicitly specified
> since the ability to connect to the pre-initialized GlobalScript is fundamentally
> an optimization.

I think this statement contradicts your goal of sharing DOM and data.

I think the "Navigation Hop" idea is actually quite tricky, and needs much more
rigorous specification.

> That only can happen in a single process so mult-process browsers like Chromium
> will need to group such pages in the same process. 

This contradicts your statement above that such pages need not be grouped in the
same process.
Comment 3 Patrick Mueller 2009-11-20 14:34:53 PST
(In reply to comment #2)
> > GlobalScript - shared, same-thread JS context
> 
> I think the word "global" is overused in JavaScript, and often misleading.
> 
> "window" is described as the "global" object, so you're creating the concept of
> a
> global global object, which encompasses a set of sub-objects which are
> themselves
> global (but not global global!).
> 
> Also, the thing you're calling "global" is not actually global, since there can
> be
> more than one of it.
> 
> This is weird. Let's not add to the pile.
> 
> How about the name SharedScript instead?
> 
> In addition to avoiding the "global" pitfall, I think "Shared" does more to
> communicate the interesting quirk that many clients can load the same
> SharedScript,
> but the script will actually load only once, and all clients will share it.

I agree "Shared" is a better word here.  But the "thing" in play here is really a new context - or environment - or scope - unsure what the proper name is.  Shared Context or Shared Scope feels better to me.

One of the aspects I like about the proposal is exactly that it gives developers another "scope" to play with.  Something you can kinda get with iframes, presumably without the overhead of the UI aspect.  In that light, I could imagine having a new way of creating a new scope, that wouldn't be shared - would be available just to a single window/document/whatever; then there would be a similar duality between worker/shared worker, and unshared scope / shared scope.

For reference, an unshared (or shared) scope could be used to implement something like the module facility provided by CommonJS - see http://wiki.commonjs.org/wiki/CommonJS for more info.
Comment 4 Dmitry Titov 2009-11-20 14:59:16 PST
Hi Geoffrey,

thanks a lot for taking a look! 

You have a convincing argument why the current naming is not good and could be
even misleading. I agree SharedScript may be even better name. Hmm... May need
to rename the bunch of things :-)

> > Currently there is no mechanism to directly share DOM, code and data on the
> > same ui thread across several pages of the web application.
> 
> This statement isn't really true, and it made it hard for me to understand your
> proposal initially.
> 
> You can share DOM, code, and data on the same UI thread across several pages
> using a window object.
> 
> I think what you're getting at here is that there's not way to do what you want
> without opening and/or persisting a user-visible window.

That's exactly right. It is very inconvenient to force a window to stay open -
apps today often use onbeforeunload dialog to prevent users from closing
windows that 'share' their state with others...

Although one can create a JS object and pass it around (and rely on GC for
lifetime), many things like timers and XHR (for example) are in fact tied to a
window/document and will stop working once 'original' window is closed. It's
theoretically possible to workaround this by detecting closures/failures and
writing even more code. But it is like saying that XHR is not necessary because
one could use iframe to load some XML and then access it from the parent.
Strictly speaking this is true, but practically it's complex and error-prone,
so XHR is better.

In case of Global(or Shared)Script, the hope is that it'll add a useful concept
that is also easy to use in practice.

> > All pages connected to the same Global Script should run on the same thread, 
> > in the same process (they logically form an 'application instance').  Since 
> > this is not always technically possible, it should be legal (and not break the 
> > applications) for there to be duplicate global script contexts within a UA.
> 
> I don't understand this. Wouldn't duplicate global script contexts break the
> feature of sharing DOM and data?

It is about multiprocess browsers, basically. As we have more of them we'll
need to solve the issue of grouping pages in processes. This is something which
is new and not yet solved - see for example the Caveats section in this doc:
http://www.chromium.org/developers/design-documents/process-models

Multiple instances of global scripts will be created if the UA happens to open
pages of the same app in different processes. This is basically like running
several instances of the native app in separate processes, they actually could
be run this way, and use different state, be logged into different accounts etc
- just like separate instances of a native app. When native apps do not want
this, they usually grab some inter-process lock and one of them shoots itself,
optionally popping another into focus. Same can be done with web apps via
SharedWorker.

The apps that currently want this (like gmail) normally use window.open() which
lets them force UA to keep the sharing pages in the same process. For today,
this works. But I can see how we could need a way to describe a set of pages
like an application, so the UA would know in advance if it needs to group them
in a single process. This will help to avoid using 'pages from the same site in
the same process' which pushes too much of unrelated things into the same
process.

> For example, if I stored a session token in a global script context, wouldn't a
> page with a duplicate global script context be forced to log in a second time,
> and then end up in a different session anyway, possible breaking the app?

As above - it may be a feature actually. Or can be avoided by the app by
bringing the already opened window in focus.

> > opening a tearoff page for the chat with 'username' (this code lives in 
> > globalScript so it can easily check for 'app-wide' info like whether or not the
> > chat window for 'username' was already open:
> 
> Notice that this example is not true if you allow for duplicate global script
> contexts, since one global script context might know if a chat window for
> 'username' were already open, while another did not.

Today, the chat windows in Gmail (which I know a bit, but other apps like that
could have similar structure) are either iframes or separate windows that are
opened via window.open(). This will ensure they have a single shared
GlobalScript. So this will work. If user opens a new tab, and navigates
separately to gmail.com - this is equivalent to opening another 'mail
application' and I personally would like to log in with different account into
another window :-)

> > Another goodness is that observed behavior does not depend on whether the
> > script is already loaded or not.
> 
> Note that observed behavior of the load and error events does depend on whether
> the script is already loaded or not, since they fire synchronously if the
> script
> is already loaded, and asynchronously otherwise. These two behaviors are
> philosophically inconsistent.

I might be not clear in the proposal - but in both cases the event fires
asynchronously, meaning the JS should yield. The fact that it fires very
quickly if script is loaded should not make those inconsistent, it's just the
timing difference.


> I think the "Navigation Hop" idea is actually quite tricky, and needs much more
> rigorous specification.

In fact, I am myself still not sure navigation hop is strictly needed, or which shape
it has to be. Some developers voiced an idea that if SharedWorker stayed a
short period after owner document dies,  then in some user scenarios there is a
high probability that the next document that needs the same SharedWorker will
come up quickly (as in navigation) and have benefit of not
creating/initialization of SharedWorker. I was thinking along the same line,
but indeed this needs more thought, initial implementation may not have one.

> > That only can happen in a single process so mult-process browsers like Chromium
> > will need to group such pages in the same process. 
> 
> This contradicts your statement above that such pages need not be grouped in
> the same process.

Agree this is not clear.  What I meant to say that the set of pages that
connect to a given GlobalScript form "unit of related browsing contexts" (as in
http://www.whatwg.org/specs/web-apps/current-work/multipage/browsers.html#unit-of-related-browsing-contexts).
There may be more then one such unit (there could be the same set of pages
opened in a separate process) so then there will be more then one set of
GlobalScripts.. Which is, again, simply as running multiple app instances.
Comment 5 Geoffrey Garen 2009-11-20 16:29:52 PST
> > I don't understand this. Wouldn't duplicate global script contexts break the
> > feature of sharing DOM and data?

> It is about multiprocess browsers, basically. As we have more of them we'll
> need to solve the issue of grouping pages in processes.

I disagree that this is a matter of multiprocess browsers. A uniprocess browser
can group pages into separate logical processes; inversely, a multiprocess browser
can merge pages into the same logical process.

Also, I think we do a disservice to web APIs if we describe their behavior too
much in terms of internal implementation constraints like process models. Those
contraints are meaningless, and potentially confusing, to web developers.

Let's not overfocus on Google properties and how they use window.open(), either,
since non-Google properties will use this API too.

I see what you mean when you mention the HTML 5 notion of a "unit of related
browsing contexts", though. Theoretically, two unrelated browsing contexts
should have unrelated SharedScripts, unrelated SharedWorkers, etc.

Let's leave the discussion of these duplicate / unrelated SharedScripts out of
the SharedScript design document, though. Or, at the very most, let's just mention
and link to the HTML 5 browsing context spec. Otherwise, it sounds like we're
inventing a new kind of browsing context relation specific to SharedScript,
with rules that possibly conflict with the HTML 5 browsing context spec.

> If user opens a new tab, and navigates separately to gmail.com - this is
> equivalent to opening another 'mail application' and I personally would like
> to log in with different account into another window :-)

I see what you mean now, and I think this equivalence, and our preference for
what should happen in a new tab opened by the user, seems best discussed in the
HTML 5 browsing context spec, and not in the SharedScript design document.

> I might be not clear in the proposal - but in both cases the event fires
> asynchronously, meaning the JS should yield. The fact that it fires very
> quickly if script is loaded should not make those inconsistent, it's just the
> timing difference.

I think I was confused by this sentence:

"If the GlobalScritp [sic] is already loaded by another page - or even by the
same page - the specified load or error events will be dispatched immediately."

If the events will actually be dispatched asynchronously, and not immediately,
I think you should just remove that sentence.

> > I think the "Navigation Hop" idea is actually quite tricky, and needs much more
> > rigorous specification.

> In fact, I am myself still not sure navigation hop is strictly needed, or which
> shape it has to be.

One use case you mentioned was "application that uses navigation from page to page".

If you intend to support this use case, the "Navigation Hop" is a requirement,
and not just an optimization. If you do not intend to support this use case,
"Navigation Hop" is just an optimization, and probably not a very good one.

I think you should either remove the "application that uses navigation from page
to page" use case and the "Navigation Hop" feature, or specify the "Navigation Hop"
feature more rigorously and remove the language about how it's just an optional
optimization. Since you don't plan to implement the "Navigation Hop" feature in
your first cut, I guess I'd lean toward the removal option.
Comment 6 Dmitry Titov 2009-11-23 11:51:55 PST
(In reply to comment #5)

Thanks Geoffrey! Your comments helped me realize that the 'spec' doc I have indeed contains way more pieces of previous discussions and implementation details then needed.

So I've split the actual spec we could use into a separate doc, renamed GlobalScript into SharedScritp and cleaned it up (removing Nav hop, discussion about browser contexts etc). Here it is:  http://docs.google.com/View?id=dxv3cth_4gvggh3gh

It might not make a lot of sense to update the patch attached to this bug (as it is a prototype) for name change, but I'll update 2 patches that are out there not yet landed for a SharedScript name change.

Thanks again!
Comment 7 Geoffrey Garen 2009-11-23 12:20:20 PST
A few comments on the JSC bindings, which generally look good:

+JSGlobalScriptContextBase::JSGlobalScriptContextBase(NonNullPassRefPtr<JSC::Structure> structure, PassRefPtr<GlobalScriptContext> impl)
+    : JSDOMGlobalObject(structure, new JSDOMGlobalObjectData, this)
+    , m_impl(impl)
+{
+}

impl can be a NonNullPassRefPtr too.

This function is so simple, maybe it should be inlined into the header.

+
+JSGlobalScriptContextBase::~JSGlobalScriptContextBase()
+{
+}

No need to include this empty destructor.

+void JSGlobalScriptContext::markChildren(MarkStack& markStack)
+{
+    Base::markChildren(markStack);
+
+    JSGlobalData& globalData = *this->globalData();
+
+    markActiveObjectsForContext(markStack, globalData, scriptExecutionContext());
+
+    impl()->markEventListeners(markStack);
+}

A little strange / verbose to put "globalData" into a local variable when it's used only once.

+    // FIXME: We need to use both the dynamic scope and the lexical scope (dynamic scope for resolving the script URL)
+    DOMWindow* window = asJSDOMWindow(exec->lexicalGlobalObject())->impl();
+    ExceptionCode ec = 0;
+    RefPtr<WebKitGlobalScript> gs = WebKitGlobalScript::create(scriptURL, name, window->document(), ec);
+    setDOMException(exec, ec);
+
+    return asObject(toJS(exec, jsConstructor->globalObject(), gs.release()));

You should write a layout test to cover this FIXME. That way, you'll know when it's fixed! :)

+GlobalScriptController::~GlobalScriptController()
+{
+    m_globalScriptContextWrapper = 0; // Unprotect the global object.
+}

This destructor is redundant with the automatic behavior of ProtectedPtr. You should remove it.
Comment 8 Dmitry Titov 2009-11-23 14:55:36 PST
Update title to reflect name change.
Comment 9 Dmitry Titov 2009-11-23 14:58:29 PST
(In reply to comment #7)

Thanks a lot! All these will be fixed in the bindings patch which will come in a separate bug soon.
Comment 10 Ian 'Hixie' Hickson 2009-11-24 05:22:57 PST
(In reply to comment #5)
> 
> I see what you mean when you mention the HTML 5 notion of a "unit of related
> browsing contexts", though. Theoretically, two unrelated browsing contexts
> should have unrelated SharedScripts, unrelated SharedWorkers, etc.

FWIW, SharedWorkers are shared by all browsing contexts, even those in different processes.
Comment 11 Geoffrey Garen 2009-11-24 12:29:03 PST
> > I see what you mean when you mention the HTML 5 notion of a "unit of related
> > browsing contexts", though. Theoretically, two unrelated browsing contexts
> > should have unrelated SharedScripts, unrelated SharedWorkers, etc.
> 
> FWIW, SharedWorkers are shared by all browsing contexts, even those in
> different processes.

Interesting! So, we did accidentally contradict the browsing context spec. Even more evidence that it was a good choice to remove that language.
Comment 12 Dmitry Titov 2009-12-07 16:59:22 PST
We are not implementing SharedScript, instead will investigate an option to keep iframe internally active while reparenting between document.

The initial code was removed from the sources.