Bug 25774 - Need to upstream V8MessagePortCustom.cpp
Summary: Need to upstream V8MessagePortCustom.cpp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: David Levin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-05-13 17:50 PDT by Andrew Wilson
Modified: 2009-05-14 12:59 PDT (History)
1 user (show)

See Also:


Attachments
patch with upstreamed file (7.52 KB, patch)
2009-05-13 17:56 PDT, Andrew Wilson
dglazkov: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Wilson 2009-05-13 17:50:31 PDT
The V8 implementation of MessagePorts needs to be upstreamed.
Comment 1 Andrew Wilson 2009-05-13 17:56:32 PDT
Created attachment 30307 [details]
patch with upstreamed file
Comment 2 Eric Seidel (no email) 2009-05-14 07:45:18 PDT
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.
Comment 3 Dimitri Glazkov (Google) 2009-05-14 08:53:23 PDT
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?
Comment 4 Andrew Wilson 2009-05-14 10:52:59 PDT
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).
Comment 5 David Levin 2009-05-14 11:16:42 PDT
Assign to levin for landing.
Comment 6 David Levin 2009-05-14 12:59:36 PDT
http://trac.webkit.org/changeset/43722