[Webkit-unassigned] [Bug 27824] [chromium] v8 binding and gyp changes for the chromium port's multi process message port implementation

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=27824





--- Comment #5 from David Levin <levin at chromium.org>  2009-07-30 10:00:48 PDT ---
(From update of attachment 33757)
Detailed comments about the remaining portions.

> Index: WebCore/ChangeLog
> +2009-07-29  John Abd-El-Malek  <jam at chromium.org>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        V8 binding and GYP changes for the Chromium port's multi process message port implementation 
> +
> +        https://bugs.webkit.org/show_bug.cgi?id=27824
> +

As Eric indicated, "Your changelog doesn't make clear what you're doing here."

There should be something here that explains what you are doing.


> +        No new tests. (OOPS!)

   > > Why no tests?
   > > +        No new tests. (OOPS!)
  > The existing layout tests cover this.  

Please change the line "No new tests. (OOPS!)" to say this.


> Index: WebCore/bindings/v8/V8GCController.cpp
> +        }
> +
> +        // Set back the weak bit that we cleared in GCPrologueVisitor.

This comment says what is being done (which I can tell from the code) but not
why it is being done.

It took me a while but I think I figured out why. Is doing this so that the
port will be attempted to 
be garbage collected again in the next round because the condition "if
(port1->isEntangled() && !port2)" may change?

And without making this weak, the garbage collection of port is never done?


> +        if (type == V8ClassIndex::MESSAGEPORT) {
> +            MessagePort* port1 = static_cast<MessagePort*>(object);
> +            MessagePort* port2 = port1->locallyEntangledPort();
> +            if (port1->isEntangled() && !port2)
> +                wrapper.MakeWeak(port1, &DOMDataStore::weakActiveDOMObjectCallback);

PS If you don't want anyone to commit your patch, simply assign the bug to
yourself, which I'll do for you now.  When you're ready for landing, coordinate
with someone to land the patch. In general, people only automatically land
patches assigned to "nodoby".

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.