Bug 32563 - [V8] Generate toV8 conversion helpers, a la JSC bindings.
Summary: [V8] Generate toV8 conversion helpers, a la JSC bindings.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nate Chapin
URL:
Keywords:
Depends on: 33677
Blocks: 32630
  Show dependency treegraph
 
Reported: 2009-12-15 09:13 PST by Dimitri Glazkov (Google)
Modified: 2010-02-04 00:40 PST (History)
6 users (show)

See Also:


Attachments
Generate toNative() converter (toV8() will be done in a later patch) (18.87 KB, patch)
2010-01-13 12:05 PST, Nate Chapin
dglazkov: review+
Details | Formatted Diff | Diff
First attempt at generating toV8() (44.42 KB, patch)
2010-01-22 14:44 PST, Nate Chapin
no flags Details | Formatted Diff | Diff
Attempt #1.1 at generating toV8() (44.44 KB, patch)
2010-01-25 09:43 PST, Nate Chapin
no flags Details | Formatted Diff | Diff
toV8() Attempt #2.0 (113.15 KB, patch)
2010-01-27 11:02 PST, Nate Chapin
no flags Details | Formatted Diff | Diff
toV8() Attempt #3.0 (108.61 KB, patch)
2010-01-27 16:51 PST, Nate Chapin
dglazkov: review-
Details | Formatted Diff | Diff
Attempt #3.1 (109.29 KB, patch)
2010-01-29 10:39 PST, Nate Chapin
no flags Details | Formatted Diff | Diff
Attempt #3.2 (110.00 KB, patch)
2010-02-01 12:32 PST, Nate Chapin
dglazkov: review+
dglazkov: commit-queue-
Details | Formatted Diff | Diff
Switch custom bindings over to toV8() (111.97 KB, patch)
2010-02-02 14:06 PST, Nate Chapin
dglazkov: review+
Details | Formatted Diff | Diff
Now without Chromium test failures! (114.17 KB, patch)
2010-02-03 14:06 PST, Nate Chapin
dglazkov: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dimitri Glazkov (Google) 2009-12-15 09:13:46 PST
This is going to be sooo pretty.
Comment 1 Nate Chapin 2010-01-13 12:05:17 PST
Created attachment 46481 [details]
Generate toNative() converter (toV8() will be done in a later patch)
Comment 2 Dimitri Glazkov (Google) 2010-01-13 12:45:08 PST
Comment on attachment 46481 [details]
Generate toNative() converter (toV8() will be done in a later patch)

Going native on V8 bindings, eh? :D
Comment 3 Nate Chapin 2010-01-13 14:11:23 PST
Comment on attachment 46481 [details]
Generate toNative() converter (toV8() will be done in a later patch)

http://trac.webkit.org/changeset/53204
Comment 4 Nate Chapin 2010-01-22 14:44:32 PST
Created attachment 47228 [details]
First attempt at generating toV8()

This very well may not be the right answer, but it's one way to generate toV8() converters.  This patch is primarily just generating the converters and making the generated bindings use them.  The custom bindings still use the old converters.

The biggest sticking point of this idea is that it moves a lot of the special cases that currently live in V8DOMWrapper.cpp into CodeGeneratorV8.pm.  I could see a case against that :)

Input appreciated.
Comment 5 WebKit Review Bot 2010-01-22 15:07:30 PST
Attachment 47228 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/206131
Comment 6 Nate Chapin 2010-01-22 15:10:51 PST
Comment on attachment 47228 [details]
First attempt at generating toV8()

Ewww.  Obsoleting while I look into what happened here.
Comment 7 Nate Chapin 2010-01-25 09:43:17 PST
Created attachment 47352 [details]
Attempt #1.1 at generating toV8()

I think (and hope) that I've accounted for the differences between compiler nits now.
Comment 8 Nate Chapin 2010-01-25 10:08:50 PST
Comment on attachment 47352 [details]
Attempt #1.1 at generating toV8()

After discussion with dglazkov, we agree that this is almost certainly not the right answer, so obsoleting this patch.
Comment 9 Nate Chapin 2010-01-27 11:02:03 PST
Created attachment 47550 [details]
toV8() Attempt #2.0
Comment 10 WebKit Review Bot 2010-01-27 11:38:04 PST
Attachment 47550 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/216152
Comment 11 Nate Chapin 2010-01-27 11:58:56 PST
Comment on attachment 47550 [details]
toV8() Attempt #2.0

I appear to have forgotten an #if ENABLED guard.
Comment 12 Nate Chapin 2010-01-27 16:51:14 PST
Created attachment 47576 [details]
toV8() Attempt #3.0

This is my current thought for handling the casting problems for HTMLElement and SVGElement. I just wanted to get it some eyeballs, since I'll be out of office tomorrow.  It changes dom/make_names.pl to support generating wrapping code for V8 (it currently is hard-coded for JSC) and therefore uses a nearly identical technique to what JSC does.
Comment 13 anton muhin 2010-01-28 04:34:49 PST
(In reply to comment #12)
> Created an attachment (id=47576) [details]
> toV8() Attempt #3.0
> 
> This is my current thought for handling the casting problems for HTMLElement
> and SVGElement. I just wanted to get it some eyeballs, since I'll be out of
> office tomorrow.  It changes dom/make_names.pl to support generating wrapping
> code for V8 (it currently is hard-coded for JSC) and therefore uses a nearly
> identical technique to what JSC does.

Several concerns.

1) object wrapping is very performance critical, so, please, verify that your patch doesn't affect performance;  and it might be better to have separation of create new vs. create on higher level (not threading the flag);

2) former V8DOMWrapper::convertNode... is pretty complicated code and lifting it as a whole into code generator looks problematic for me.  For example, any experiment would lead to rebuild all and compile-run cycle would be very long. Overall I'd prefer to keep generated non-trivial code at the minimum.  Is it possible to keep most of conversion logic in plain C++ and only invoke necessary bits from codegen?  For example:

 1983   if (V8IsolatedContext::getEntered())
 1984     proxy = 0;
 1985   else if (!proxy)
 1986     proxy = V8Proxy::retrieve();
 1987 
 1988   if (proxy)
 1989     wrapper = proxy->windowShell()->createWrapperFromCache(${wrapperType});
 1990   else
 1991     wrapper = SafeAllocation::newInstance(V8DOMWrapper::getTemplate(${wrapperType})->GetFunction());
 1992   if (!wrapper.IsEmpty())
 1993     V8DOMWrapper::setDOMWrapper(wrapper, V8ClassIndex::ToInt(${wrapperType}), impl);
 1994 END
 1995 

This looks like a perfect inline function on its own.

3)

 sub HasCustomToV8Implementation {
 2043     $dataNode = shift;
 2044     $interfaceName = shift;
 2045     
 2046     # We generate a custom converter (but JSC doesn't) for the following:
 2047     return 1 if $interfaceName eq "BarInfo";
 2048     return 1 if $interfaceName eq "CSSStyleSheet";
 2049     return 1 if $interfaceName eq "CanvasPixelArray";
 2050     return 1 if $interfaceName eq "Console";
 2051     return 1 if $interfaceName eq "DOMSelection";
 2052     return 1 if $interfaceName eq "DOMWindow";
 2053     return 1 if $interfaceName eq "Element";
 2054     return 1 if $interfaceName eq "Location";
 2055     return 1 if $interfaceName eq "HTMLDocument";
 2056     return 1 if $interfaceName eq "HTMLElement";
 2057     return 1 if $interfaceName eq "History";
 2058     return 1 if $interfaceName eq "NamedNodeMap";
 2059     return 1 if $interfaceName eq "Navigator";
 2060     return 1 if $interfaceName eq "SVGDocument";
 2061     return 1 if $interfaceName eq "SVGElement";
 2062     return 1 if $interfaceName eq "Screen";
 2063     
 2064     # We don't generate a custom converter (but JSC does) for the following:
 2065     return 0 if $interfaceName eq "AbstractWorker";
 2066     return 0 if $interfaceName eq "CanvasRenderingContext";
 2067     return 0 if $interfaceName eq "HTMLCollection";
 2068     return 0 if $interfaceName eq "ImageData";
 2069     return 0 if $interfaceName eq "SVGElementInstance";
 2070 
 2071     # For everything else, do what JSC does.
 2072     return $dataNode->extendedAttributes->{"CustomToJS"};
 2073 }
 2074 

Shouldn't that be an attribute in IDL?

Same question for

 2088 sub IsActiveDomType
 2089 {
 2090     my $type = shift;
 2091     return 1 if $type eq "MessagePort";
 2092     return 1 if $type eq "XMLHttpRequest";
 2093     return 1 if $type eq "WebSocket";
 2094     return 1 if $type eq "Worker";
 2095     return 1 if $type eq "SharedWorker";
 2096     return 0;
 2097 }

4) This change from double to float, is it fine?  Could it be done in a separate CL?

5)

 1942   //if (!impl)
 1943   //  return v8::Null();

should be probably removed.

And thanks a lot for keeping me in the loop.
Comment 14 Dimitri Glazkov (Google) 2010-01-28 10:21:01 PST
Comment on attachment 47576 [details]
toV8() Attempt #3.0

Overall, I think the 3.0 is definitely better than the 1.0, but Anton has good comments and I'll go with him.
Comment 15 Nate Chapin 2010-01-29 10:38:28 PST
(In reply to comment #13)
> 1) object wrapping is very performance critical, so, please, verify that your
> patch doesn't affect performance;  and it might be better to have separation of
> create new vs. create on higher level (not threading the flag);
> 

Will do on the perf tests.  For the separation of new vs. existing objects, I don't think it's feasible.  Typically, we start with a return type of something like "Element" for items that are labelled "ReturnsNew" in an IDL.  So, e.g., toV8(Element*) is calling toV8(HTMLElement*) is calling toV8(HTMLSomeOtherKindOfElement*), and I think it's going to make the code even uglier than it is currently to have a separate "create new object" code path.

> 2) former V8DOMWrapper::convertNode... is pretty complicated code and lifting
> it as a whole into code generator looks problematic for me.  For example, any
> experiment would lead to rebuild all and compile-run cycle would be very long.
> Overall I'd prefer to keep generated non-trivial code at the minimum.  Is it
> possible to keep most of conversion logic in plain C++ and only invoke
> necessary bits from codegen?  For example:
> 
>  1983   if (V8IsolatedContext::getEntered())
>  1984     proxy = 0;
>  1985   else if (!proxy)
>  1986     proxy = V8Proxy::retrieve();
>  1987 
>  1988   if (proxy)
>  1989     wrapper =
> proxy->windowShell()->createWrapperFromCache(${wrapperType});
>  1990   else
>  1991     wrapper =
> SafeAllocation::newInstance(V8DOMWrapper::getTemplate(${wrapperType})->GetFunction());
>  1992   if (!wrapper.IsEmpty())
>  1993     V8DOMWrapper::setDOMWrapper(wrapper,
> V8ClassIndex::ToInt(${wrapperType}), impl);
>  1994 END
>  1995 
> 
> This looks like a perfect inline function on its own.

Yeah, I got experimenting with generating that.  I think inlining is the right answer after all.  That block is precisely what V8DOMWrapper::instantiateV8Object() does.

> 
> 3)
> 
>  sub HasCustomToV8Implementation {
>  2043     $dataNode = shift;
>  2044     $interfaceName = shift;
>  2045     
>  2046     # We generate a custom converter (but JSC doesn't) for the following:
>  2047     return 1 if $interfaceName eq "BarInfo";
>  2048     return 1 if $interfaceName eq "CSSStyleSheet";
>  2049     return 1 if $interfaceName eq "CanvasPixelArray";
>  2050     return 1 if $interfaceName eq "Console";
>  2051     return 1 if $interfaceName eq "DOMSelection";
>  2052     return 1 if $interfaceName eq "DOMWindow";
>  2053     return 1 if $interfaceName eq "Element";
>  2054     return 1 if $interfaceName eq "Location";
>  2055     return 1 if $interfaceName eq "HTMLDocument";
>  2056     return 1 if $interfaceName eq "HTMLElement";
>  2057     return 1 if $interfaceName eq "History";
>  2058     return 1 if $interfaceName eq "NamedNodeMap";
>  2059     return 1 if $interfaceName eq "Navigator";
>  2060     return 1 if $interfaceName eq "SVGDocument";
>  2061     return 1 if $interfaceName eq "SVGElement";
>  2062     return 1 if $interfaceName eq "Screen";
>  2063     
>  2064     # We don't generate a custom converter (but JSC does) for the
> following:
>  2065     return 0 if $interfaceName eq "AbstractWorker";
>  2066     return 0 if $interfaceName eq "CanvasRenderingContext";
>  2067     return 0 if $interfaceName eq "HTMLCollection";
>  2068     return 0 if $interfaceName eq "ImageData";
>  2069     return 0 if $interfaceName eq "SVGElementInstance";
>  2070 
>  2071     # For everything else, do what JSC does.
>  2072     return $dataNode->extendedAttributes->{"CustomToJS"};
>  2073 }
>  2074 
> 
> Shouldn't that be an attribute in IDL?
> 
> Same question for
> 
>  2088 sub IsActiveDomType
>  2089 {
>  2090     my $type = shift;
>  2091     return 1 if $type eq "MessagePort";
>  2092     return 1 if $type eq "XMLHttpRequest";
>  2093     return 1 if $type eq "WebSocket";
>  2094     return 1 if $type eq "Worker";
>  2095     return 1 if $type eq "SharedWorker";
>  2096     return 0;
>  2097 }

Adding FIXMEs for these.  That may be the right long-term solution.

> 
> 4) This change from double to float, is it fine?  Could it be done in a
> separate CL?

We're pretty loose with double<->float conversion in here, and my changes caused some of those to become compile errors, so I figured standardizing made sense.  All the layout tests still pass, but if there's something else you would like me to do to verify that this change is fine, let me know.

> 
> 5)
> 
>  1942   //if (!impl)
>  1943   //  return v8::Null();
> 
> should be probably removed.

Doh.

> 
> And thanks a lot for keeping me in the loop.

Next patch will be uploaded momentarily.
Comment 16 Nate Chapin 2010-01-29 10:39:19 PST
Created attachment 47721 [details]
Attempt #3.1
Comment 17 anton muhin 2010-02-01 04:02:31 PST
(In reply to comment #15)
> (In reply to comment #13)
> > 1) object wrapping is very performance critical, so, please, verify that your
> > patch doesn't affect performance;  and it might be better to have separation of
> > create new vs. create on higher level (not threading the flag);
> > 
> 
> Will do on the perf tests.  For the separation of new vs. existing objects, I
> don't think it's feasible.  Typically, we start with a return type of something
> like "Element" for items that are labelled "ReturnsNew" in an IDL.  So, e.g.,
> toV8(Element*) is calling toV8(HTMLElement*) is calling
> toV8(HTMLSomeOtherKindOfElement*), and I think it's going to make the code even
> uglier than it is currently to have a separate "create new object" code path.
>

From my experience, this separation is really important.  I don't quite understand why it cannot be implemented---after all the main difference is absence of check if object is already wrapped or not.  Maybe I am missing some context here.

> > 2) former V8DOMWrapper::convertNode... is pretty complicated code and lifting
> > it as a whole into code generator looks problematic for me.  For example, any
> > experiment would lead to rebuild all and compile-run cycle would be very long.
> > Overall I'd prefer to keep generated non-trivial code at the minimum.  Is it
> > possible to keep most of conversion logic in plain C++ and only invoke
> > necessary bits from codegen?  For example:
> > 
> >  1983   if (V8IsolatedContext::getEntered())
> >  1984     proxy = 0;
> >  1985   else if (!proxy)
> >  1986     proxy = V8Proxy::retrieve();
> >  1987 
> >  1988   if (proxy)
> >  1989     wrapper =
> > proxy->windowShell()->createWrapperFromCache(${wrapperType});
> >  1990   else
> >  1991     wrapper =
> > SafeAllocation::newInstance(V8DOMWrapper::getTemplate(${wrapperType})->GetFunction());
> >  1992   if (!wrapper.IsEmpty())
> >  1993     V8DOMWrapper::setDOMWrapper(wrapper,
> > V8ClassIndex::ToInt(${wrapperType}), impl);
> >  1994 END
> >  1995 
> > 
> > This looks like a perfect inline function on its own.
> 
> Yeah, I got experimenting with generating that.  I think inlining is the right
> answer after all.  That block is precisely what
> V8DOMWrapper::instantiateV8Object() does.
> 
> > 
> > 3)
> > 
> >  sub HasCustomToV8Implementation {
> >  2043     $dataNode = shift;
> >  2044     $interfaceName = shift;
> >  2045     
> >  2046     # We generate a custom converter (but JSC doesn't) for the following:
> >  2047     return 1 if $interfaceName eq "BarInfo";
> >  2048     return 1 if $interfaceName eq "CSSStyleSheet";
> >  2049     return 1 if $interfaceName eq "CanvasPixelArray";
> >  2050     return 1 if $interfaceName eq "Console";
> >  2051     return 1 if $interfaceName eq "DOMSelection";
> >  2052     return 1 if $interfaceName eq "DOMWindow";
> >  2053     return 1 if $interfaceName eq "Element";
> >  2054     return 1 if $interfaceName eq "Location";
> >  2055     return 1 if $interfaceName eq "HTMLDocument";
> >  2056     return 1 if $interfaceName eq "HTMLElement";
> >  2057     return 1 if $interfaceName eq "History";
> >  2058     return 1 if $interfaceName eq "NamedNodeMap";
> >  2059     return 1 if $interfaceName eq "Navigator";
> >  2060     return 1 if $interfaceName eq "SVGDocument";
> >  2061     return 1 if $interfaceName eq "SVGElement";
> >  2062     return 1 if $interfaceName eq "Screen";
> >  2063     
> >  2064     # We don't generate a custom converter (but JSC does) for the
> > following:
> >  2065     return 0 if $interfaceName eq "AbstractWorker";
> >  2066     return 0 if $interfaceName eq "CanvasRenderingContext";
> >  2067     return 0 if $interfaceName eq "HTMLCollection";
> >  2068     return 0 if $interfaceName eq "ImageData";
> >  2069     return 0 if $interfaceName eq "SVGElementInstance";
> >  2070 
> >  2071     # For everything else, do what JSC does.
> >  2072     return $dataNode->extendedAttributes->{"CustomToJS"};
> >  2073 }
> >  2074 
> > 
> > Shouldn't that be an attribute in IDL?
> > 
> > Same question for
> > 
> >  2088 sub IsActiveDomType
> >  2089 {
> >  2090     my $type = shift;
> >  2091     return 1 if $type eq "MessagePort";
> >  2092     return 1 if $type eq "XMLHttpRequest";
> >  2093     return 1 if $type eq "WebSocket";
> >  2094     return 1 if $type eq "Worker";
> >  2095     return 1 if $type eq "SharedWorker";
> >  2096     return 0;
> >  2097 }
> 
> Adding FIXMEs for these.  That may be the right long-term solution.
> 
> > 
> > 4) This change from double to float, is it fine?  Could it be done in a
> > separate CL?
> 
> We're pretty loose with double<->float conversion in here, and my changes
> caused some of those to become compile errors, so I figured standardizing made
> sense.  All the layout tests still pass, but if there's something else you
> would like me to do to verify that this change is fine, let me know.

Makes sense to me, thanks for clarifications.  I don't know how you feel about it, but this unification might be better done with a separate change, before this change goes in---it's already complicated enough.  But that's completely up to you.

New batch:

1)
 842             } else {
 843                 print F <<END

tabs got into your files---beware!

2) current state of toV8Helper might have performance implications---you could lookup a map to put object into twice---first time to get a map to check if wrapper is present and second time to add a wrapper into the map.  You might be better storing this map into a local variable and reuse it later (cf. to the current implementation.)

Note as well, that recently another fast check was added for the case of the Node---we check a pointer of the object itself (please let me know if you need more details.)

3)

112          ASSERT(V8DOMWrapper::domWrapperType(info.Holder()) == V8ClassIndex::NODE);

Why this is removed?

4)

 196     default: break; // XPATH_NAMESPACE_NODE

What does this comment mean?

5)
79      ASSERT(V8DOMWrapper::domWrapperType(info.Holder()) == V8ClassIndex::NODE);

Ditto, why removed?

6) this

 61     v8::Handle<v8::Object> wrapper = getDOMObjectMap().get(impl);
 62     if (wrapper.IsEmpty()) {
 63         wrapper = V8Console::toV8Helper(impl);
 64         if (!wrapper.IsEmpty())
 65             V8DOMWrapper::setHiddenWindowReference(impl->frame(), V8DOMWindow::consoleIndex, wrapper);
 66     }

 and owner handling seems a pretty common pattern.  Maybe generalize it in this CL?  Or you'd better postpone it for some further time?

7)

 90     bindings/v8/custom/V8BarInfoCustom.cpp \
 91     bindings/v8/custom/V8CSSRuleCustom.cpp \

here and below, why additional indent?
Comment 18 Nate Chapin 2010-02-01 11:24:24 PST
Comment on attachment 47721 [details]
Attempt #3.1

Obsoleting this patch, I need to make a couple more minor changes.
Comment 19 Nate Chapin 2010-02-01 12:32:44 PST
Created attachment 47864 [details]
Attempt #3.2

The two big changes in this version are:
1. Renamed toV8Helper() to wrap()
2. Integrated http://trac.webkit.org/changeset/53944 into the generated node wrapping functions.
Comment 20 Dimitri Glazkov (Google) 2010-02-01 12:47:49 PST
Comment on attachment 47864 [details]
Attempt #3.2

Yay! r=me, after fixing:

> +    switch (impl->pathSegType()) {
> +    case SVGPathSeg::PATHSEG_CLOSEPATH:                    return toV8(static_cast<SVGPathSegClosePath*>(impl));

I know you copied this over, but this needs to be in WebKit style, with line breaks/indents.

> Index: WebCore/Android.v8bindings.mk
> ===================================================================
> --- WebCore/Android.v8bindings.mk	(revision 54136)
> +++ WebCore/Android.v8bindings.mk	(working copy)
> @@ -87,8 +87,13 @@
>  	\
>  	bindings/v8/custom/V8AbstractWorkerCustom.cpp \
>  	bindings/v8/custom/V8AttrCustom.cpp \
> +    bindings/v8/custom/V8BarInfoCustom.cpp \
> +    bindings/v8/custom/V8CSSRuleCustom.cpp \
>  	bindings/v8/custom/V8CSSStyleDeclarationCustom.cpp \
> +    bindings/v8/custom/V8CSSStyleSheetCustom.cpp \
> +    bindings/v8/custom/V8CSSValueCustom.cpp \
>  	bindings/v8/custom/V8CanvasRenderingContext2DCustom.cpp \
> +    bindings/v8/custom/V8CanvasPixelArrayCustom.cpp \

Tabs in this file?
Comment 21 Nate Chapin 2010-02-02 08:29:24 PST
Comment on attachment 47864 [details]
Attempt #3.2

Landed in http://trac.webkit.org/changeset/54150 and http://trac.webkit.org/changeset/54153. Leaving bug open because we still need to switch the custom bindings over to using toV8().
Comment 22 Nate Chapin 2010-02-02 14:06:17 PST
Created attachment 47974 [details]
Switch custom bindings over to toV8()

Will run perf tests locally momentarily.
Comment 23 Dimitri Glazkov (Google) 2010-02-02 14:49:21 PST
Comment on attachment 47974 [details]
Switch custom bindings over to toV8()

Hurray! I thereby deem this the coolest patch eva.
Comment 24 Nate Chapin 2010-02-02 15:24:39 PST
http://trac.webkit.org/changeset/54259
Comment 25 Shinichiro Hamaji 2010-02-03 05:02:28 PST
Committed r54278: <http://trac.webkit.org/changeset/54278>
Comment 26 Shinichiro Hamaji 2010-02-03 05:50:42 PST
I reverted this change as it seems to break chromium's unit tests. This is an example of the failures:

http://build.chromium.org/buildbot/try-server/buildstatus?builder=win&number=17123
Comment 27 Nate Chapin 2010-02-03 14:06:17 PST
Created attachment 48067 [details]
Now without Chromium test failures!

Two changes:
1. Added some code to CodeGeneratorV8.pm to use WorkerContextExecutionProxy when necessary to actually do the wrapping instead of V8DOMWrapper.
2. Fixed a bug where I was incorrectly returning early in allowPopUp() in V8DOMWindowCustom.cpp.
Comment 28 Dimitri Glazkov (Google) 2010-02-03 14:10:08 PST
Comment on attachment 48067 [details]
Now without Chromium test failures!

ok. Make sure to check w/ukai about WebSockets + Workers.
Comment 29 Nate Chapin 2010-02-03 14:35:00 PST
Landed (again): http://trac.webkit.org/changeset/54305
Comment 30 Shinichiro Hamaji 2010-02-04 00:40:55 PST
(In reply to comment #29)
> Landed (again): http://trac.webkit.org/changeset/54305

It seems the roll succeeded this time. Thanks for the fix!