Web Inspector: forced pseudo classes aren't cleared from inspected page when Inspecto...
authorjoepeck@webkit.org <joepeck@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 5 Jan 2017 04:37:43 +0000 (04:37 +0000)
committerjoepeck@webkit.org <joepeck@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 5 Jan 2017 04:37:43 +0000 (04:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=108823
<rdar://problem/13143550>

Reviewed by Timothy Hatcher.

* inspector/InspectorCSSAgent.h:
* inspector/InspectorCSSAgent.cpp:
(WebCore::InspectorCSSAgent::documentDetached):
Clear the document from all of the different Document sets.

(WebCore::InspectorCSSAgent::didRemoveDocument): Deleted.
Use documentDetached, which is more direct.

(WebCore::InspectorCSSAgent::forcePseudoState):
Update the set of Documents with psuedo element changes. So when we
reset forced styles we know which documents to refresh styles.

(WebCore::InspectorCSSAgent::resetPseudoStates):
Use the list of documents we've already computed.

(WebCore::InspectorCSSAgent::didRemoveDOMNode):
(WebCore::InspectorCSSAgent::didModifyDOMAttr):
Change to take a reference and more data to avoid extra work.

* inspector/InspectorDOMAgent.h:
* inspector/InspectorDOMAgent.cpp:
(WebCore::InspectorDOMAgent::unbind):
Eliminated didRemoveDocument.

(WebCore::InspectorDOMAgent::didModifyDOMAttr):
(WebCore::InspectorDOMAgent::didRemoveDOMAttr):
(WebCore::InspectorDOMAgent::styleAttributeInvalidated):
Pass a references to the DOM listener client, these are never null.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@210316 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/inspector/InspectorCSSAgent.cpp
Source/WebCore/inspector/InspectorCSSAgent.h
Source/WebCore/inspector/InspectorDOMAgent.cpp
Source/WebCore/inspector/InspectorDOMAgent.h

index 211837d..03b78ba 100644 (file)
@@ -1,3 +1,40 @@
+2017-01-04  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: forced pseudo classes aren't cleared from inspected page when Inspector closes
+        https://bugs.webkit.org/show_bug.cgi?id=108823
+        <rdar://problem/13143550>
+
+        Reviewed by Timothy Hatcher.
+
+        * inspector/InspectorCSSAgent.h:
+        * inspector/InspectorCSSAgent.cpp:
+        (WebCore::InspectorCSSAgent::documentDetached):
+        Clear the document from all of the different Document sets.
+
+        (WebCore::InspectorCSSAgent::didRemoveDocument): Deleted.
+        Use documentDetached, which is more direct.
+
+        (WebCore::InspectorCSSAgent::forcePseudoState):
+        Update the set of Documents with psuedo element changes. So when we
+        reset forced styles we know which documents to refresh styles.
+
+        (WebCore::InspectorCSSAgent::resetPseudoStates):
+        Use the list of documents we've already computed.
+
+        (WebCore::InspectorCSSAgent::didRemoveDOMNode):
+        (WebCore::InspectorCSSAgent::didModifyDOMAttr):
+        Change to take a reference and more data to avoid extra work.
+
+        * inspector/InspectorDOMAgent.h:
+        * inspector/InspectorDOMAgent.cpp:
+        (WebCore::InspectorDOMAgent::unbind):
+        Eliminated didRemoveDocument.
+
+        (WebCore::InspectorDOMAgent::didModifyDOMAttr):
+        (WebCore::InspectorDOMAgent::didRemoveDOMAttr):
+        (WebCore::InspectorDOMAgent::styleAttributeInvalidated):
+        Pass a references to the DOM listener client, these are never null.
+
 2017-01-04  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Remove runtime flag for variation fonts
index 3d54221..0589474 100644 (file)
@@ -410,6 +410,8 @@ void InspectorCSSAgent::documentDetached(Document& document)
     setActiveStyleSheetsForDocument(document, emptyList);
 
     m_documentToKnownCSSStyleSheets.remove(&document);
+    m_documentToInspectorStyleSheet.remove(&document);
+    m_documentsWithForcedPseudoStates.remove(&document);
 }
 
 void InspectorCSSAgent::mediaQueryResultChanged()
@@ -536,7 +538,7 @@ bool InspectorCSSAgent::forcePseudoState(const Element& element, CSSSelector::Ps
     if (!nodeId)
         return false;
 
-    NodeIdToForcedPseudoState::iterator it = m_nodeIdToForcedPseudoState.find(nodeId);
+    auto it = m_nodeIdToForcedPseudoState.find(nodeId);
     if (it == m_nodeIdToForcedPseudoState.end())
         return false;
 
@@ -880,17 +882,21 @@ void InspectorCSSAgent::forcePseudoState(ErrorString& errorString, int nodeId, c
     if (!element)
         return;
 
+    auto it = m_nodeIdToForcedPseudoState.find(nodeId);
     unsigned forcedPseudoState = computePseudoClassMask(forcedPseudoClasses);
-    NodeIdToForcedPseudoState::iterator it = m_nodeIdToForcedPseudoState.find(nodeId);
     unsigned currentForcedPseudoState = it == m_nodeIdToForcedPseudoState.end() ? 0 : it->value;
-    bool needStyleRecalc = forcedPseudoState != currentForcedPseudoState;
-    if (!needStyleRecalc)
+    if (forcedPseudoState == currentForcedPseudoState)
         return;
 
-    if (forcedPseudoState)
+    if (forcedPseudoState) {
         m_nodeIdToForcedPseudoState.set(nodeId, forcedPseudoState);
-    else
+        m_documentsWithForcedPseudoStates.add(&element->document());
+    } else {
         m_nodeIdToForcedPseudoState.remove(nodeId);
+        if (m_nodeIdToForcedPseudoState.isEmpty())
+            m_documentsWithForcedPseudoStates.clear();
+    }
+
     element->document().styleScope().didChangeStyleSheetEnvironment();
 }
 
@@ -1042,12 +1048,12 @@ RefPtr<Inspector::Protocol::CSS::CSSRule> InspectorCSSAgent::buildObjectForRule(
     return inspectorStyleSheet ? inspectorStyleSheet->buildObjectForRule(rule, nullptr) : nullptr;
 }
 
-RefPtr<Inspector::Protocol::Array<Inspector::Protocol::CSS::RuleMatch>> InspectorCSSAgent::buildArrayForMatchedRuleList(const Vector<RefPtr<StyleRule>>& matchedRules, StyleResolver& styleResolver, Element& element, PseudoId psuedoId)
+RefPtr<Inspector::Protocol::Array<Inspector::Protocol::CSS::RuleMatch>> InspectorCSSAgent::buildArrayForMatchedRuleList(const Vector<RefPtr<StyleRule>>& matchedRules, StyleResolver& styleResolver, Element& element, PseudoId pseudoId)
 {
     auto result = Inspector::Protocol::Array<Inspector::Protocol::CSS::RuleMatch>::create();
 
     SelectorChecker::CheckingContext context(SelectorChecker::Mode::CollectingRules);
-    context.pseudoId = psuedoId ? psuedoId : element.pseudoId();
+    context.pseudoId = pseudoId ? pseudoId : element.pseudoId();
     SelectorChecker selectorChecker(element.document());
 
     for (auto& matchedRule : matchedRules) {
@@ -1149,35 +1155,21 @@ RefPtr<Inspector::Protocol::CSS::NamedFlow> InspectorCSSAgent::buildObjectForNam
         .release();
 }
 
-void InspectorCSSAgent::didRemoveDocument(Document* document)
-{
-    if (document)
-        m_documentToInspectorStyleSheet.remove(document);
-}
-
-void InspectorCSSAgent::didRemoveDOMNode(Node* node)
+void InspectorCSSAgent::didRemoveDOMNode(Node& node, int nodeId)
 {
-    if (!node)
-        return;
-
-    int nodeId = m_domAgent->boundNodeId(node);
-    if (nodeId)
-        m_nodeIdToForcedPseudoState.remove(nodeId);
+    m_nodeIdToForcedPseudoState.remove(nodeId);
 
-    NodeToInspectorStyleSheet::iterator it = m_nodeToInspectorStyleSheet.find(node);
+    auto it = m_nodeToInspectorStyleSheet.find(&node);
     if (it == m_nodeToInspectorStyleSheet.end())
         return;
 
     m_idToInspectorStyleSheet.remove(it->value->id());
-    m_nodeToInspectorStyleSheet.remove(node);
+    m_nodeToInspectorStyleSheet.remove(&node);
 }
 
-void InspectorCSSAgent::didModifyDOMAttr(Element* element)
+void InspectorCSSAgent::didModifyDOMAttr(Element& element)
 {
-    if (!element)
-        return;
-
-    NodeToInspectorStyleSheet::iterator it = m_nodeToInspectorStyleSheet.find(element);
+    auto it = m_nodeToInspectorStyleSheet.find(&element);
     if (it == m_nodeToInspectorStyleSheet.end())
         return;
 
@@ -1191,15 +1183,11 @@ void InspectorCSSAgent::styleSheetChanged(InspectorStyleSheet* styleSheet)
 
 void InspectorCSSAgent::resetPseudoStates()
 {
-    HashSet<Document*> documentsToChange;
-    for (auto& nodeId : m_nodeIdToForcedPseudoState) {
-        if (Element* element = downcast<Element>(m_domAgent->nodeForId(nodeId.key)))
-            documentsToChange.add(&element->document());
-    }
+    for (auto& document : m_documentsWithForcedPseudoStates)
+        document->styleScope().didChangeStyleSheetEnvironment();
 
     m_nodeIdToForcedPseudoState.clear();
-    for (auto& document : documentsToChange)
-        document->styleScope().didChangeStyleSheetEnvironment();
+    m_documentsWithForcedPseudoStates.clear();
 }
 
 } // namespace WebCore
index 8d1cc41..78ebca7 100644 (file)
@@ -158,9 +158,8 @@ private:
     RefPtr<Inspector::Protocol::CSS::NamedFlow> buildObjectForNamedFlow(ErrorString&, WebKitNamedFlow*, int documentNodeId);
 
     // InspectorDOMAgent::DOMListener implementation
-    void didRemoveDocument(Document*) override;
-    void didRemoveDOMNode(Node*) override;
-    void didModifyDOMAttr(Element*) override;
+    void didRemoveDOMNode(Node&, int nodeId) override;
+    void didModifyDOMAttr(Element&) override;
 
     // InspectorCSSAgent::Listener implementation
     void styleSheetChanged(InspectorStyleSheet*) override;
@@ -177,6 +176,7 @@ private:
     DocumentToViaInspectorStyleSheet m_documentToInspectorStyleSheet;
     HashMap<Document*, HashSet<CSSStyleSheet*>> m_documentToKnownCSSStyleSheets;
     NodeIdToForcedPseudoState m_nodeIdToForcedPseudoState;
+    HashSet<Document*> m_documentsWithForcedPseudoStates;
     HashSet<int> m_namedFlowCollectionsRequested;
     std::unique_ptr<ChangeRegionOversetTask> m_changeRegionOversetTask;
 
index 3860ef3..df961e0 100644 (file)
@@ -328,10 +328,7 @@ void InspectorDOMAgent::unbind(Node* node, NodeToIdMap* nodesMap)
 
     if (node->isFrameOwnerElement()) {
         const HTMLFrameOwnerElement* frameOwner = static_cast<const HTMLFrameOwnerElement*>(node);
-        Document* contentDocument = frameOwner->contentDocument();
-        if (m_domListener)
-            m_domListener->didRemoveDocument(contentDocument);
-        if (contentDocument)
+        if (Document* contentDocument = frameOwner->contentDocument())
             unbind(contentDocument, nodesMap);
     }
 
@@ -347,7 +344,7 @@ void InspectorDOMAgent::unbind(Node* node, NodeToIdMap* nodesMap)
 
     nodesMap->remove(node);
     if (m_domListener)
-        m_domListener->didRemoveDOMNode(node);
+        m_domListener->didRemoveDOMNode(*node, id);
 
     bool childrenRequested = m_childrenRequested.contains(id);
     if (childrenRequested) {
@@ -2021,7 +2018,7 @@ void InspectorDOMAgent::didModifyDOMAttr(Element& element, const AtomicString& n
         return;
 
     if (m_domListener)
-        m_domListener->didModifyDOMAttr(&element);
+        m_domListener->didModifyDOMAttr(element);
 
     m_frontendDispatcher->attributeModified(id, name, value);
 }
@@ -2034,7 +2031,7 @@ void InspectorDOMAgent::didRemoveDOMAttr(Element& element, const AtomicString& n
         return;
 
     if (m_domListener)
-        m_domListener->didModifyDOMAttr(&element);
+        m_domListener->didModifyDOMAttr(element);
 
     m_frontendDispatcher->attributeRemoved(id, name);
 }
@@ -2049,7 +2046,7 @@ void InspectorDOMAgent::styleAttributeInvalidated(const Vector<Element*>& elemen
             continue;
 
         if (m_domListener)
-            m_domListener->didModifyDOMAttr(element);
+            m_domListener->didModifyDOMAttr(*element);
         nodeIds->addItem(id);
     }
     m_frontendDispatcher->inlineStyleInvalidated(WTFMove(nodeIds));
index 8d65b55..89bc75e 100644 (file)
@@ -92,12 +92,9 @@ class InspectorDOMAgent final : public InspectorAgentBase, public Inspector::DOM
     WTF_MAKE_FAST_ALLOCATED;
 public:
     struct DOMListener {
-        virtual ~DOMListener()
-        {
-        }
-        virtual void didRemoveDocument(Document*) = 0;
-        virtual void didRemoveDOMNode(Node*) = 0;
-        virtual void didModifyDOMAttr(Element*) = 0;
+        virtual ~DOMListener() { }
+        virtual void didRemoveDOMNode(Node&, int nodeId) = 0;
+        virtual void didModifyDOMAttr(Element&) = 0;
     };
 
     InspectorDOMAgent(WebAgentContext&, InspectorPageAgent*, InspectorOverlay*);