[webkit-reviews] review granted: [Bug 26623] [Chromium] Upstream V8Proxy : [Attachment 31893] patch5

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has granted Nate Chapin <japhet at google.com>'s
request for review:
Bug 26623: [Chromium] Upstream V8Proxy
https://bugs.webkit.org/show_bug.cgi?id=26623

Attachment 31893: patch5
https://bugs.webkit.org/attachment.cgi?id=31893&action=review

------- Additional Comments from David Levin <levin at chromium.org>
I think it has made huge progress and we should land it.

Please fix this *before* landing (just fix it and land it *without* uploading
another patch):

  > WebCore/bindings/v8/V8DOMMap.cpp

   Has unresolved conflict in it at line 495.





Ideally we'd have a *subsequent* patch to fix the following few things: 
(Please *don't* update this patch with these fixes.)
  
  > WebCore/bindings/v8/V8Proxy.cpp


  1. Finish getting rid of USE_VAR and replace with UNUSED_PARAM.
  2. In all classes, public: protected: private: shouldn't be indented.
  3. In class GrouperItem,
     a. the variable names need to have this m_style
     b. group_id should be groupId (for the method and variable)
     c. For the constructor: "Each member (and superclass) should be indented
on a separate line"
  4. In several of the class the functions have braces at the end of line.
  5. In ObjectGrouperVisitor::visitDOMWrapper
     a. typo "elment"
     b. braces around single line statement
  6. ObjectGrouperVisitor::grouper_ naming.
  7. JavaScriptConsoleMessage::JavaScriptConsoleMessage constructor braces are
too indented.
  8. ConsoleMessageManager::processDelayedMessages braces on single line
statement.
  9. There are several places that do the two spaces before end of line
comments (should be one) -- search for ";  //"
  10. V8Proxy::setContextDebugId has end of line brace.