Bug 25774 – Need to upstream V8MessagePortCustom.cpp

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Andrew Wilson]

The V8 implementation of MessagePorts needs to be upstreamed.

[Andrew Wilson]

Created attachment 30307 [details]
patch with upstreamed file

[Eric Seidel (no email)]

Comment on attachment 30307 [details]
patch with upstreamed file

It seems we could share some code between the onMessage and onClose handlers no?  I think I would have used a static function to make those long if/else blocks not need to be copy/paste there.  Please feel free to re-mark r=? if I'm missing something.

Otherwise the code looks sane enough.  createHiddenDependency and removeHiddenDependency seem confusing... what are they for?

I assume also that this is already covered by existing layout tests?  Otherwise this needs some form of testing.

[Dimitri Glazkov (Google)]

Comment on attachment 30307 [details]
patch with upstreamed file

Eric is probably right about making this look better, but right now let's stay on the goal of moving all these upstream. Drew, can you file a WebKit bug to track (and act on) Eric's suggestion?

[Andrew Wilson]

I've logged bug #25795 to track the needed refactoring. The required refactoring is actually very widespread - the code pattern Eric mentioned is shared across many different files in binding/v8/custom and we should refactor all of them.

I believe that createHiddenDependency() and removeHiddenDependency() (from V8Utilities.h) are standard utility routines used by v8 to track the event listeners for GC purposes (make sure they don't get GC'd prematurely).

[David Levin]

Assign to levin for landing.

[David Levin]

http://trac.webkit.org/changeset/43722