[Webkit-unassigned] [Bug 42886] Web Inspector: implement DOM breakpoints

[bugzilla-daemon at webkit.org]

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


Yury Semikhatsky <yurys at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #64945|review?                     |review-
               Flag|                            |




--- Comment #25 from Yury Semikhatsky <yurys at chromium.org>  2010-08-20 05:08:16 PST ---
(From update of attachment 64945)
WebCore/inspector/InspectorDOMAgent.cpp:1066
 +      if (!node)
You don't seem to need this check.

WebCore/inspector/InspectorDOMAgent.cpp:1064
 +  void InspectorDOMAgent::updateSubtreeBreakpoints(Node* node, long mask, bool set)
I'd rename mask to rootMask or something that would reflect the fact that it's not shifted.

WebCore/inspector/InspectorDOMAgent.h:218
 +          HashMap<Node*, long> m_breakpoints;
Why not have int as value type? We use only 32 bits in the code anyway.

WebCore/inspector/InspectorDOMAgent.cpp:1060
 +      return s_domAgentOnBreakpoint != 0;
Please reset s_domAgentOnBreakpoint to 0 before leaving this method.

WebCore/bindings/v8/ScriptDebugServer.cpp:243
 +      v8::Handle<v8::Context> context = v8::Context::GetCurrent();
Isn't it Debugger context?

Pleas address the comments, otherwise looks good.

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