[Webkit-unassigned] [Bug 27421] [V8] Factor V8ConsoleMessage out of V8Proxy

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Adam Barth <abarth at webkit.org>  2009-07-19 19:55:22 PDT ---
> > Index: WebCore/bindings/v8/V8ConsoleMessage.cpp
> > +    for (int i = 0; i < size; ++i)
> > +        delayedMessages->at(i).dispatchNow(page);
> 
> I'm concerned about the call to dispatchNow because it in turn calls
> V8ConsoleMessage::processDelayed.
> 
> At first, I thought this was ok because no new messages should get added to the
> console but then I saw this note about dispatchNow
>    48         // Add a message to the console. May end up calling JavaScript
> code
>    49         // indirectly through the inspector so only call this function
> when
>    50         // it is safe to do allocations.
> so it looks like it can end up running JavaScript (which may result in added
> messages?).

I think this stuff is all fine.  The only one we have to worry about is
|dispatchLater|.  We're not supposed to call JavaScript while running that
function.  Both dispatchNow and processDelayed are supposed to be called when
it's safe to do allocations.

> > + }  // namespace WebCore
> 
> Extra space before the // here.

Fixed.

> > Index: WebCore/bindings/v8/V8ConsoleMessage.h
> > +        static void handler(v8::Handle<v8::Message> message, v8::Handle<v8::Value> data);
> 
> "message" param name looks redundant.

Fixed.

> > +        // All delayed messages are stored in this vector. If the vector
> > +        // is NULL, there are no delayed messages.
> 
> Since no NULLs are in the code, it would be good just to make this comment say
> "0" as well.

Fixed.

> > + }
> 
> Change to "} // namespace WebCore"

Fixed.

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