[webkit-reviews] review canceled: [Bug 44679] Web Inspector: provide more information to front-end when breaking on DOM event : [Attachment 65909] Proposed patch.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 30 06:43:55 PDT 2010


Yury Semikhatsky <yurys at chromium.org> has canceled Pavel Podivilov
<podivilov at chromium.org>'s request for review:
Bug 44679: Web Inspector: provide more information to front-end when breaking
on DOM event
https://bugs.webkit.org/show_bug.cgi?id=44679

Attachment 65909: Proposed patch.
https://bugs.webkit.org/attachment.cgi?id=65909&action=review

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 908f761..925163a 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,59 @@
> +2010-08-30  Pavel Podivilov	<podivilov at chromium.org>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Web Inspector: provide more information to front-end when breaking
on DOM event
> +	   https://bugs.webkit.org/show_bug.cgi?id=44679
> +
> +	   * dom/ContainerNode.cpp:
> +	   (WebCore::ContainerNode::insertBefore):
> +	   (WebCore::ContainerNode::parserInsertBefore):
> +	   (WebCore::ContainerNode::replaceChild):
> +	   (WebCore::ContainerNode::appendChild):
> +	   (WebCore::ContainerNode::parserAddChild):
> +	   (WebCore::notifyChildInserted):
> +	   (WebCore::dispatchChildRemovalEvents):
> +	   * dom/Element.cpp:
> +	   (WebCore::Element::setAttribute):
> +	   (WebCore::Element::removeAttribute):
> +	   * inspector/Inspector.idl:
> +	   * inspector/InspectorController.h:
> +	   (WebCore::InspectorController::willInsertDOMNode):
> +	   (WebCore::InspectorController::didInsertDOMNode):
> +	   (WebCore::InspectorController::willRemoveDOMNode):
> +	   (WebCore::InspectorController::willModifyDOMAttr):
> +	   (WebCore::InspectorController::didModifyDOMAttr):
> +	   (WebCore::InspectorController::inspectorControllerForNode):
> +	   * inspector/InspectorDOMAgent.cpp:
> +	   (WebCore::InspectorDOMAgent::~InspectorDOMAgent):
> +	   (WebCore::InspectorDOMAgent::getBreakpointForNodeInsertion):
> +	   (WebCore::InspectorDOMAgent::getBreakpointForNodeRemoval):
> +	   (WebCore::InspectorDOMAgent::getBreakpointForAttributeModification):

> +	   (WebCore::InspectorDOMAgent::didInsertDOMNode):
> +	   (WebCore::InspectorDOMAgent::didRemoveDOMNode):
> +	   (WebCore::InspectorDOMAgent::didModifyDOMAttr):
> +	   (WebCore::InspectorDOMAgent::createBreakpoint):
> +	   * inspector/InspectorDOMAgent.h:
> +	   * inspector/InspectorDebuggerAgent.cpp:
> +	   (WebCore::InspectorDebuggerAgent::InspectorDebuggerAgent):
> +	   (WebCore::InspectorDebuggerAgent::~InspectorDebuggerAgent):
> +	   (WebCore::InspectorDebuggerAgent::didPause):
> +	   (WebCore::InspectorDebuggerAgent::breakProgram):
> +	   * inspector/InspectorDebuggerAgent.h:
> +	   * inspector/InspectorValues.h:
> +	   (WebCore::InspectorValue::isNull):
> +	   * inspector/front-end/BreakpointsSidebarPane.js:
> +	   (WebInspector.BreakpointItem):
> +	   (WebInspector.BreakpointItem.prototype._enableChanged):
> +	   * inspector/front-end/DOMAgent.js:
> +	   (WebInspector.DOMAgent.prototype._childNodeRemoved):
> +	   (WebInspector.DOMBreakpointManager.prototype._breakpointRemoved):
> +	   (WebInspector.DOMBreakpointManager.prototype.breakpointHit):
> +	   * inspector/front-end/Script.js:
> +	   (WebInspector.Script.prototype.get linesCount):
> +	   * inspector/front-end/inspector.js:
> +	   (WebInspector.pausedScript):
> +
>  2010-08-28  Jeremy Moskovich  <jeremy at chromium.org>
>  
>	   Reviewed by Dimitri Glazkov.
> diff --git a/WebCore/dom/ContainerNode.cpp b/WebCore/dom/ContainerNode.cpp
> index ef62b38..7d88948 100644
> --- a/WebCore/dom/ContainerNode.cpp
> +++ b/WebCore/dom/ContainerNode.cpp
> @@ -134,6 +134,8 @@ bool ContainerNode::insertBefore(PassRefPtr<Node>
newChild, Node* refChild, Exce
>	   if (child->parentNode())
>	       break;
>  
> +	   InspectorController::willInsertDOMNode(child, this);
> +
>	   insertBeforeCommon(next.get(), child);
>  
>	   // Send notification about the children change.
> @@ -201,6 +203,8 @@ void ContainerNode::parserInsertBefore(PassRefPtr<Node>
newChild, Node* nextChil
>      for (NodeVector::const_iterator it = targets.begin(); it !=
targets.end(); ++it) {
>	   Node* child = it->get();
>  
> +	   InspectorController::willInsertDOMNode(child, this);
> +
>	   insertBeforeCommon(next.get(), child);
>  
>	   childrenChanged(true, nextChildPreviousSibling.get(), nextChild, 1);

> @@ -274,6 +278,8 @@ bool ContainerNode::replaceChild(PassRefPtr<Node>
newChild, Node* oldChild, Exce
>	   ASSERT(!child->nextSibling());
>	   ASSERT(!child->previousSibling());
>  
> +	   InspectorController::willInsertDOMNode(child.get(), this);
> +
>	   // Add child after "prev".
>	   forbidEventDispatch();
>	   Node* next;
> @@ -555,6 +561,8 @@ bool ContainerNode::appendChild(PassRefPtr<Node>
newChild, ExceptionCode& ec, bo
>		   break;
>	   }
>  
> +	   InspectorController::willInsertDOMNode(child, this);
> +
>	   // Append child to the end of the list
>	   forbidEventDispatch();
>	   child->setParent(this);
> @@ -593,6 +601,8 @@ void ContainerNode::parserAddChild(PassRefPtr<Node>
newChild)
>      ASSERT(newChild);
>      ASSERT(!newChild->parent()); // Use appendChild if you need to handle
reparenting (and want DOM mutation events).
>  
> +    InspectorController::willInsertDOMNode(newChild.get(), this);
> +
>      forbidEventDispatch();
>      Node* last = m_lastChild;
>      // FIXME: This method should take a PassRefPtr.
> @@ -964,12 +974,7 @@ static void notifyChildInserted(Node* child)
>  {
>      ASSERT(!eventDispatchForbidden());
>  
> -#if ENABLE(INSPECTOR)
> -    if (Page* page = child->document()->page()) {
> -	   if (InspectorController* inspectorController =
page->inspectorController())
> -	       inspectorController->didInsertDOMNode(child);
> -    }
> -#endif
> +    InspectorController::didInsertDOMNode(child);
>  
>      RefPtr<Node> c = child;
>      RefPtr<Document> document = child->document();
> @@ -1003,12 +1008,7 @@ static void dispatchChildRemovalEvents(Node* child)
>  {
>      ASSERT(!eventDispatchForbidden());
>  
> -#if ENABLE(INSPECTOR)    
> -    if (Page* page = child->document()->page()) {
> -	   if (InspectorController* inspectorController =
page->inspectorController())
> -	       inspectorController->didRemoveDOMNode(child);
> -    }
> -#endif
> +    InspectorController::willRemoveDOMNode(child);
>  
>      RefPtr<Node> c = child;
>      RefPtr<Document> document = child->document();
> diff --git a/WebCore/dom/Element.cpp b/WebCore/dom/Element.cpp
> index 39bf393..fb17565 100644
> --- a/WebCore/dom/Element.cpp
> +++ b/WebCore/dom/Element.cpp
> @@ -540,6 +540,11 @@ void Element::setAttribute(const AtomicString& name,
const AtomicString& value,
>	   return;
>      }
>  
> +#if ENABLE(INSPECTOR)
> +    if (!isSynchronizingStyleAttribute())
> +	   InspectorController::willModifyDOMAttr(this);
> +#endif
> +
>      const AtomicString& localName = shouldIgnoreAttributeCase(this) ?
name.lower() : name;
>  
>      // Allocate attribute map if necessary.
> @@ -563,17 +568,18 @@ void Element::setAttribute(const AtomicString& name,
const AtomicString& value,
>      }
>  
>  #if ENABLE(INSPECTOR)
> -    if (Page* page = document()->page()) {
> -	   if (InspectorController* inspectorController =
page->inspectorController()) {
> -	       if (!isSynchronizingStyleAttribute())
> -		   inspectorController->didModifyDOMAttr(this);
> -	   }
> -    }
> +    if (!isSynchronizingStyleAttribute())
> +	   InspectorController::didModifyDOMAttr(this);
>  #endif
>  }
>  
>  void Element::setAttribute(const QualifiedName& name, const AtomicString&
value, ExceptionCode&)
>  {
> +#if ENABLE(INSPECTOR)
> +    if (!isSynchronizingStyleAttribute())
> +	   InspectorController::willModifyDOMAttr(this);
> +#endif
> +
>      document()->incDOMTreeVersion();
>  
>      // Allocate attribute map if necessary.
> @@ -592,12 +598,8 @@ void Element::setAttribute(const QualifiedName& name,
const AtomicString& value,
>      }
>  
>  #if ENABLE(INSPECTOR)
> -    if (Page* page = document()->page()) {
> -	   if (InspectorController* inspectorController =
page->inspectorController()) {
> -	       if (!isSynchronizingStyleAttribute())
> -		   inspectorController->didModifyDOMAttr(this);
> -	   }
> -    }
> +    if (!isSynchronizingStyleAttribute())
> +	   InspectorController::didModifyDOMAttr(this);
>  #endif
>  }
>  
> @@ -1228,6 +1230,8 @@ void Element::setAttributeNS(const AtomicString&
namespaceURI, const AtomicStrin
>  
>  void Element::removeAttribute(const String& name, ExceptionCode& ec)
>  {
> +    InspectorController::willModifyDOMAttr(this);
> +
>      String localName = shouldIgnoreAttributeCase(this) ? name.lower() :
name;
>  
>      if (m_attributeMap) {
> @@ -1236,13 +1240,7 @@ void Element::removeAttribute(const String& name,
ExceptionCode& ec)
>	       ec = 0;
>      }
>      
> -#if ENABLE(INSPECTOR)
> -    if (Page* page = document()->page()) {
> -	   if (InspectorController* inspectorController =
page->inspectorController())
> -	       inspectorController->didModifyDOMAttr(this);
> -    }
> -#endif
> -    
> +    InspectorController::didModifyDOMAttr(this);
>  }
>  
>  void Element::removeAttributeNS(const String& namespaceURI, const String&
localName, ExceptionCode& ec)
> diff --git a/WebCore/inspector/Inspector.idl
b/WebCore/inspector/Inspector.idl
> index 1f034b2..f58ae67 100644
> --- a/WebCore/inspector/Inspector.idl
> +++ b/WebCore/inspector/Inspector.idl
> @@ -72,7 +72,7 @@ module core {
>	   [notify] void debuggerWasDisabled();
>	   [notify] void failedToParseScriptSource(out String url, out String
data, out int firstLine, out int errorLine, out String errorMessage);
>	   [notify] void parsedScriptSource(out String sourceID, out String
url, out String data, out int firstLine, out int scriptWorldType);
> -	   [notify] void pausedScript(out Value callFrames);
> +	   [notify] void pausedScript(out Value callFrames, out Value details);

Instead of adding additional parameter change the Value parameter to Object and
put the callFrames along with the details into it.


>	   [notify] void profilerWasEnabled();
>	   [notify] void profilerWasDisabled();
>	   [notify] void restoredBreakpoint(out String sourceID, out String
url, out int line, out boolean enabled, out String condition);
> diff --git a/WebCore/inspector/InspectorController.h
b/WebCore/inspector/InspectorController.h
> index 7ed2549..f9981a6 100644
> --- a/WebCore/inspector/InspectorController.h
> +++ b/WebCore/inspector/InspectorController.h
> @@ -31,7 +31,10 @@
>  
>  #include "Console.h"
>  #include "Cookie.h"
> +#include "Element.h"
>  #include "InspectorDOMAgent.h"
> +#include "InspectorDebuggerAgent.h"
> +#include "Page.h"
>  #include "PlatformString.h"
>  #include "ScriptState.h"
>  #include <wtf/HashMap.h>
> @@ -60,7 +63,6 @@ class InspectorClient;
>  class InspectorCSSStore;
>  class InspectorDOMStorageResource;
>  class InspectorDatabaseResource;
> -class InspectorDebuggerAgent;
>  class InspectorFrontend;
>  class InspectorFrontendClient;
>  class InspectorObject;
> @@ -183,9 +185,13 @@ public:
>      void mainResourceFiredLoadEvent(DocumentLoader*, const KURL&);
>      void mainResourceFiredDOMContentEvent(DocumentLoader*, const KURL&);
>  
> -    void didInsertDOMNode(Node*);
> -    void didRemoveDOMNode(Node*);
> -    void didModifyDOMAttr(Element*);
> +    static void willInsertDOMNode(Node* node, Node* parent);
> +    static void didInsertDOMNode(Node*);
> +    static void willRemoveDOMNode(Node*);
> +    static void willModifyDOMAttr(Element*);
> +    static void didModifyDOMAttr(Element*);
> +    static InspectorController* inspectorControllerForNode(Node*);
This method should be private.


> +
>  #if ENABLE(WORKERS)
>      enum WorkerAction { WorkerCreated, WorkerDestroyed };
>  
> @@ -376,30 +382,84 @@ private:
>  #endif
>  };
>  
> +inline void InspectorController::willInsertDOMNode(Node* node, Node* parent)

> +{
> +#if ENABLE(JAVASCRIPT_DEBUGGER)
> +    if (InspectorController* inspectorController =
inspectorControllerForNode(node)) {
> +	   InspectorDebuggerAgent* debuggerAgent =
inspectorController->debuggerAgent();
> +	   if (!debuggerAgent)
> +	       return;
> +	   InspectorDOMAgent* domAgent = inspectorController->domAgent();
> +	   if (!domAgent)
> +	       return;
> +	   RefPtr<InspectorValue> breakpoint =
domAgent->getBreakpointForNodeInsertion(node, parent);
> +	   if (!breakpoint->isNull())
> +	       debuggerAgent->breakProgram(breakpoint);
> +    }
> +#endif
> +}
> +
>  inline void InspectorController::didInsertDOMNode(Node* node)
>  {
>  #if ENABLE(INSPECTOR)
> -    if (m_domAgent)
> -	   m_domAgent->didInsertDOMNode(node);
> +    if (InspectorController* inspectorController =
inspectorControllerForNode(node)) {
> +	   if (InspectorDOMAgent* domAgent = inspectorController->domAgent())
> +	       domAgent->didInsertDOMNode(node);
> +    }
>  #endif
>  }
>  
> -inline void InspectorController::didRemoveDOMNode(Node* node)
> +inline void InspectorController::willRemoveDOMNode(Node* node)
>  {
> -#if ENABLE(INSPECTOR)
> -    if (m_domAgent)
> -	   m_domAgent->didRemoveDOMNode(node);
> +#if ENABLE(JAVASCRIPT_DEBUGGER)
> +    if (InspectorController* inspectorController =
inspectorControllerForNode(node)) {
> +	   InspectorDOMAgent* domAgent = inspectorController->domAgent();
> +	   if (!domAgent)
> +	       return;
> +	   if (InspectorDebuggerAgent* debuggerAgent =
inspectorController->debuggerAgent()) {
> +	       RefPtr<InspectorValue> breakpoint =
domAgent->getBreakpointForNodeRemoval(node);
> +	       if (!breakpoint->isNull())
> +		   debuggerAgent->breakProgram(breakpoint);
> +	   }
> +	   domAgent->didRemoveDOMNode(node);
> +    }
> +#endif
> +}
> +
> +inline void InspectorController::willModifyDOMAttr(Element* element)
> +{
> +#if ENABLE(JAVASCRIPT_DEBUGGER)
> +    if (InspectorController* inspectorController =
inspectorControllerForNode(element)) {
> +	   InspectorDebuggerAgent* debuggerAgent =
inspectorController->debuggerAgent();
> +	   if (!debuggerAgent)
> +	       return;
> +	   InspectorDOMAgent* domAgent = inspectorController->domAgent();
> +	   if (!domAgent)
> +	       return;
> +	   RefPtr<InspectorValue> breakpoint =
domAgent->getBreakpointForAttributeModification(element);
> +	   if (!breakpoint->isNull())
> +	       debuggerAgent->breakProgram(breakpoint);
> +    }
>  #endif
>  }
>  
>  inline void InspectorController::didModifyDOMAttr(Element* element)
>  {
>  #if ENABLE(INSPECTOR)
> -    if (m_domAgent)
> -	   m_domAgent->didModifyDOMAttr(element);
> +    if (InspectorController* inspectorController =
inspectorControllerForNode(element)) {
> +	   if (InspectorDOMAgent* domAgent = inspectorController->domAgent())
> +	       domAgent->didModifyDOMAttr(element);
> +    }
>  #endif
>  }
>  
> +inline InspectorController*
InspectorController::inspectorControllerForNode(Node* node)
> +{
> +    if (Page* page = node->document()->page())
> +	   return page->inspectorController();
> +    return 0;
> +}
> +
>  } // namespace WebCore
>  
>  #endif // !defined(InspectorController_h)
> diff --git a/WebCore/inspector/InspectorDOMAgent.cpp
b/WebCore/inspector/InspectorDOMAgent.cpp
> index 82827bd..17e957e 100644
> --- a/WebCore/inspector/InspectorDOMAgent.cpp
> +++ b/WebCore/inspector/InspectorDOMAgent.cpp
> @@ -64,7 +64,6 @@
>  #include "PlatformString.h"
>  #include "RenderStyle.h"
>  #include "RenderStyleConstants.h"
> -#include "ScriptDebugServer.h"
>  #include "ScriptEventListener.h"
>  #include "StyleSheetList.h"
>  #include "Text.h"
> @@ -210,8 +209,6 @@ const int domBreakpointDerivedTypeShift = 16;
>  
>  }
>  
> -InspectorDOMAgent* InspectorDOMAgent::s_domAgentOnBreakpoint = 0;
> -
>  InspectorDOMAgent::InspectorDOMAgent(InspectorCSSStore* cssStore,
InspectorFrontend* frontend)
>      : EventListener(InspectorDOMAgentType)
>      , m_cssStore(cssStore)
> @@ -224,9 +221,6 @@ InspectorDOMAgent::InspectorDOMAgent(InspectorCSSStore*
cssStore, InspectorFront
>  InspectorDOMAgent::~InspectorDOMAgent()
>  {
>      reset();
> -
> -    if (this == s_domAgentOnBreakpoint)
> -	   s_domAgentOnBreakpoint = 0;
>  }
>  
>  void InspectorDOMAgent::reset()
> @@ -780,6 +774,29 @@ void InspectorDOMAgent::removeDOMBreakpoint(long nodeId,
long type)
>      }
>  }
>  
> +PassRefPtr<InspectorValue>
InspectorDOMAgent::getBreakpointForNodeInsertion(Node*, Node* parent)
> +{
> +    if (hasBreakpoint(parent, SubtreeModified))
> +	   return createBreakpoint(parent, SubtreeModified);
> +    return InspectorObject::null();
> +}
> +
> +PassRefPtr<InspectorValue>
InspectorDOMAgent::getBreakpointForNodeRemoval(Node* node)
> +{
> +    if (hasBreakpoint(node, NodeRemoved))
> +	   return createBreakpoint(node, NodeRemoved);
> +    if (hasBreakpoint(innerParentNode(node), SubtreeModified))
> +	   return createBreakpoint(innerParentNode(node), SubtreeModified);
> +    return InspectorObject::null();
> +}
> +
> +PassRefPtr<InspectorValue>
InspectorDOMAgent::getBreakpointForAttributeModification(Element* element)
> +{
> +    if (hasBreakpoint(element, AttributeModified))
> +	   return createBreakpoint(element, AttributeModified);
> +    return InspectorObject::null();
> +}
> +
>  String InspectorDOMAgent::documentURLString(Document* document) const
>  {
>      if (!document || document->url().isNull())
> @@ -985,12 +1002,7 @@ void InspectorDOMAgent::didInsertDOMNode(Node* node)
>	   return;
>  
>      if (m_breakpoints.size()) {
> -	   Node* parent = innerParentNode(node);
> -	   if (hasBreakpoint(parent, SubtreeModified)) {
> -	       if (!pauseOnBreakpoint())
> -		   return;
> -	   }
> -	   uint32_t mask = m_breakpoints.get(parent);
> +	   uint32_t mask = m_breakpoints.get(innerParentNode(node));
>	   uint32_t inheritableTypesMask = (mask | (mask >>
domBreakpointDerivedTypeShift)) & inheritableDOMBreakpointTypesMask;
>	   if (inheritableTypesMask)
>	       updateSubtreeBreakpoints(node, inheritableTypesMask, true);
> @@ -1023,10 +1035,6 @@ void InspectorDOMAgent::didRemoveDOMNode(Node* node)
>	   return;
>  
>      if (m_breakpoints.size()) {
> -	   if (hasBreakpoint(node, NodeRemoved) ||
hasBreakpoint(innerParentNode(node), SubtreeModified)) {
> -	       if (!pauseOnBreakpoint())
> -		   return;
> -	   }
>	   // Remove subtree breakpoints.
>	   m_breakpoints.remove(node);
>	   Vector<Node*> stack(1, innerFirstChild(node));
> @@ -1063,12 +1071,24 @@ void InspectorDOMAgent::didModifyDOMAttr(Element*
element)
>      if (!id)
>	   return;
>  
> -    if (hasBreakpoint(element, AttributeModified)) {
> -	   if (!pauseOnBreakpoint())
> -	       return;
> +    m_frontend->attributesUpdated(id,
buildArrayForElementAttributes(element));
> +}
> +
> +PassRefPtr<InspectorObject> InspectorDOMAgent::createBreakpoint(Node* node,
long type)
> +{
> +    RefPtr<InspectorObject> breakpoint = InspectorObject::create();
> +
> +    // Find breakpoint owner.
> +    while (!(m_breakpoints.get(node) & (1 << type))) {
> +	   node = innerParentNode(node);
> +	   ASSERT(node);
>      }
> +    long nodeId = m_documentNodeToIdMap.get(node);
> +    ASSERT(nodeId);
>  
> -    m_frontend->attributesUpdated(id,
buildArrayForElementAttributes(element));
> +    breakpoint->setNumber("nodeId", nodeId);
> +    breakpoint->setNumber("type", type);
> +    return breakpoint.release();
>  }
>  
>  bool InspectorDOMAgent::hasBreakpoint(Node* node, long type)
> @@ -1078,19 +1098,6 @@ bool InspectorDOMAgent::hasBreakpoint(Node* node, long
type)
>      return m_breakpoints.get(node) & (rootBit | derivedBit);
>  }
>  
> -bool InspectorDOMAgent::pauseOnBreakpoint()
> -{
> -#if ENABLE(JAVASCRIPT_DEBUGGER)
> -    s_domAgentOnBreakpoint = this;
> -    ScriptDebugServer::shared().breakProgram();
> -    bool deleted = !s_domAgentOnBreakpoint;
> -    s_domAgentOnBreakpoint = 0;
> -    return !deleted;
> -#else
> -    return true;
> -#endif
> -}
> -
>  void InspectorDOMAgent::updateSubtreeBreakpoints(Node* node, uint32_t
rootMask, bool set)
>  {
>      uint32_t oldMask = m_breakpoints.get(node);
> diff --git a/WebCore/inspector/InspectorDOMAgent.h
b/WebCore/inspector/InspectorDOMAgent.h
> index fd3c5b5..2787802 100644
> --- a/WebCore/inspector/InspectorDOMAgent.h
> +++ b/WebCore/inspector/InspectorDOMAgent.h
> @@ -36,7 +36,6 @@
>  #include "InspectorCSSStore.h"
>  #include "InspectorValues.h"
>  #include "NodeList.h"
> -#include "ScriptState.h"
>  #include "Timer.h"
>  
>  #include <wtf/Deque.h>
> @@ -114,6 +113,9 @@ namespace WebCore {
>	   void searchCanceled();
>	   void setDOMBreakpoint(long nodeId, long type);
>	   void removeDOMBreakpoint(long nodeId, long type);
> +	   PassRefPtr<InspectorValue> getBreakpointForNodeInsertion(Node* node,
Node* parent);
> +	   PassRefPtr<InspectorValue> getBreakpointForNodeRemoval(Node*);
> +	   PassRefPtr<InspectorValue>
getBreakpointForAttributeModification(Element*);
>  
>	   // Methods called from the frontend for CSS styles inspection.
>	   void getStyles(long nodeId, bool authorOnly, RefPtr<InspectorValue>*
styles);
> @@ -160,7 +162,7 @@ namespace WebCore {
>	   bool pushDocumentToFrontend();
>  
>	   bool hasBreakpoint(Node* node, long type);
> -	   bool pauseOnBreakpoint();
> +	   PassRefPtr<InspectorObject> createBreakpoint(Node* node, long type);

>	   void updateSubtreeBreakpoints(Node* root, uint32_t rootMask, bool
value);
>  
>	   PassRefPtr<InspectorObject> buildObjectForAttributeStyles(Element*
element);
> @@ -217,8 +219,6 @@ namespace WebCore {
>	   HashSet<RefPtr<Node> > m_searchResults;
>	   Vector<long> m_inspectedNodes;
>	   HashMap<Node*, uint32_t> m_breakpoints;
> -
> -	   static InspectorDOMAgent* s_domAgentOnBreakpoint;
>      };
>  
>  #endif
> diff --git a/WebCore/inspector/InspectorDebuggerAgent.cpp
b/WebCore/inspector/InspectorDebuggerAgent.cpp
> index 357a043..afba025 100644
> --- a/WebCore/inspector/InspectorDebuggerAgent.cpp
> +++ b/WebCore/inspector/InspectorDebuggerAgent.cpp
> @@ -56,11 +56,14 @@ PassOwnPtr<InspectorDebuggerAgent>
InspectorDebuggerAgent::create(InspectorContr
>      return agent.release();
>  }
>  
> +InspectorDebuggerAgent* InspectorDebuggerAgent::s_debuggerAgentOnBreakpoint
= 0;
> +
>  InspectorDebuggerAgent::InspectorDebuggerAgent(InspectorController*
inspectorController, InspectorFrontend* frontend)
>      : m_inspectorController(inspectorController)
>      , m_frontend(frontend)
>      , m_pausedScriptState(0)
>      , m_breakpointsLoaded(false)
> +    , m_breakProgramDetails(InspectorValue::null())
>  {
>  }
>  
> @@ -68,6 +71,9 @@ InspectorDebuggerAgent::~InspectorDebuggerAgent()
>  {
>      ScriptDebugServer::shared().removeListener(this,
m_inspectorController->inspectedPage());
>      m_pausedScriptState = 0;
> +
> +    if (this == s_debuggerAgentOnBreakpoint)
> +	   s_debuggerAgentOnBreakpoint = 0;
>  }
>  
>  bool InspectorDebuggerAgent::isDebuggerAlwaysEnabled()
> @@ -283,8 +289,7 @@ void InspectorDebuggerAgent::didPause(ScriptState*
scriptState)
>  {
>      ASSERT(scriptState && !m_pausedScriptState);
>      m_pausedScriptState = scriptState;
> -    RefPtr<InspectorValue> callFrames = currentCallFrames();
> -    m_frontend->pausedScript(callFrames.get());
> +    m_frontend->pausedScript(currentCallFrames(), m_breakProgramDetails);
>  }
>  
>  void InspectorDebuggerAgent::didContinue()
> @@ -293,6 +298,19 @@ void InspectorDebuggerAgent::didContinue()
>      m_frontend->resumedScript();
>  }
>  
> +void InspectorDebuggerAgent::breakProgram(PassRefPtr<InspectorValue>
details)
> +{
> +    s_debuggerAgentOnBreakpoint = this;
> +    m_breakProgramDetails = details;
> +
> +    ScriptDebugServer::shared().breakProgram();
> +    if (!s_debuggerAgentOnBreakpoint)
> +	   return;
> +
> +    s_debuggerAgentOnBreakpoint = 0;
> +    m_breakProgramDetails = InspectorValue::null();
> +}
> +
>  } // namespace WebCore
>  
>  #endif // ENABLE(JAVASCRIPT_DEBUGGER)
> diff --git a/WebCore/inspector/InspectorDebuggerAgent.h
b/WebCore/inspector/InspectorDebuggerAgent.h
> index 91bcd49..6d6e8bc 100644
> --- a/WebCore/inspector/InspectorDebuggerAgent.h
> +++ b/WebCore/inspector/InspectorDebuggerAgent.h
> @@ -61,6 +61,7 @@ public:
>      void getScriptSource(const String& sourceID, String* scriptSource);
>  
>      void pause();
> +    void breakProgram(PassRefPtr<InspectorValue> details);
>      void resume();
>      void stepOverStatement();
>      void stepIntoStatement();
> @@ -93,6 +94,8 @@ private:
>      HashMap<String, SourceBreakpoints> m_stickyBreakpoints;
>      HashMap<String, unsigned> m_breakpointsMapping;
>      bool m_breakpointsLoaded;
> +    static InspectorDebuggerAgent* s_debuggerAgentOnBreakpoint;
> +    RefPtr<InspectorValue> m_breakProgramDetails;
>  };
>  
>  } // namespace WebCore
> diff --git a/WebCore/inspector/InspectorValues.h
b/WebCore/inspector/InspectorValues.h
> index 3dd9594..4036f55 100644
> --- a/WebCore/inspector/InspectorValues.h
> +++ b/WebCore/inspector/InspectorValues.h
> @@ -67,6 +67,8 @@ public:
>  
>      Type type() const { return m_type; }
>  
> +    bool isNull() const { return m_type == TypeNull; }
> +
>      virtual bool asBoolean(bool* output) const;
>      virtual bool asNumber(double* output) const;
>      virtual bool asNumber(long* output) const;
> diff --git a/WebCore/inspector/front-end/BreakpointsSidebarPane.js
b/WebCore/inspector/front-end/BreakpointsSidebarPane.js
> index 3a0860f..29be646 100644
> --- a/WebCore/inspector/front-end/BreakpointsSidebarPane.js
> +++ b/WebCore/inspector/front-end/BreakpointsSidebarPane.js
> @@ -106,7 +106,7 @@ WebInspector.BreakpointItem = function(breakpoint)
>      this._element.appendChild(checkboxElement);
>  
>      this._breakpoint.addEventListener("enable-changed", this._enableChanged,
this);
> -    this._breakpoint.addEventListener("removed", this._removed, this);
> +    this._breakpoint.addEventListener("removed",
this.dispatchEventToListeners.bind(this, "removed"));
>  }
>  
>  WebInspector.BreakpointItem.prototype = {
> @@ -128,15 +128,10 @@ WebInspector.BreakpointItem.prototype = {
>	   event.stopPropagation();
>      },
>  
> -    _enableChanged: function()
> +    _enableChanged: function(event)
>      {
>	   var checkbox = this._element.firstChild;
>	   checkbox.checked = this._breakpoint.enabled;
> -    },
> -
> -    _removed: function()
> -    {
> -	   this.dispatchEventToListeners("removed");
>      }
>  }
>  
> diff --git a/WebCore/inspector/front-end/DOMAgent.js
b/WebCore/inspector/front-end/DOMAgent.js
> index 5aaa0d3..e2c880a 100644
> --- a/WebCore/inspector/front-end/DOMAgent.js
> +++ b/WebCore/inspector/front-end/DOMAgent.js
> @@ -417,7 +417,7 @@ WebInspector.DOMAgent.prototype = {
>	   parent.removeChild_(node);
>	   var event = { target : node, relatedNode : parent };
>	   this.document._fireDomEvent("DOMNodeRemoved", event);
> -	   delete this._idToDOMNode[nodeId];
> +	   delete this._idToDOMNode[node.id];
Why is that change?


>      }
>  }
>  
> @@ -719,6 +719,13 @@ WebInspector.DOMBreakpointManager.prototype = {
>	   for (var type in nodeBreakpoints)
>	       return;
>	   delete this._breakpoints[breakpoint.node.id];
> +    },
> +
> +    breakpointHit: function(nodeId, type)
> +    {
> +	   var nodeBreakpoints = this._breakpoints[nodeId];
> +	   if (type in nodeBreakpoints)
> +	       nodeBreakpoints[type].dispatchEventToListeners("hit");
>      }
>  }
>  
> diff --git a/WebCore/inspector/front-end/Script.js
b/WebCore/inspector/front-end/Script.js
> index 42d6850..2ef9328 100644
> --- a/WebCore/inspector/front-end/Script.js
> +++ b/WebCore/inspector/front-end/Script.js
> @@ -62,11 +62,12 @@ WebInspector.Script.prototype = {
>      {
>	   if (!this.source)
>	       return 0;
> -	   this._linesCount = 0;
> +	   var linesCount = 0;
I think the value was supposed to be cached. Instead of changing it into a
local variable you should check whether it's been calculated.



>	   var lastIndex = this.source.indexOf("\n");
>	   while (lastIndex !== -1) {
>	       lastIndex = this.source.indexOf("\n", lastIndex + 1)
> -	       this._linesCount++;
> +	       linesCount++;
>	   }
> +	   return linesCount;
>      }
>  }
> diff --git a/WebCore/inspector/front-end/inspector.js
b/WebCore/inspector/front-end/inspector.js
> index 26dfa09..11cbe98 100644
> --- a/WebCore/inspector/front-end/inspector.js
> +++ b/WebCore/inspector/front-end/inspector.js
> @@ -1445,9 +1445,12 @@ WebInspector.failedToParseScriptSource =
function(sourceURL, source, startingLin
>      this.panels.scripts.addScript(null, sourceURL, source, startingLine,
errorLine, errorMessage);
>  }
>  
> -WebInspector.pausedScript = function(callFrames)
> +WebInspector.pausedScript = function(callFrames, details)
>  {
>      this.panels.scripts.debuggerPaused(callFrames);
> +    if (details)
> +	   this.domBreakpointManager.breakpointHit(details.nodeId,
details.type);
> +
>      InspectorFrontendHost.bringToFront();
>  }
>


More information about the webkit-reviews mailing list