Changeset 153872 in webkit

Timestamp:

Aug 8, 2013, 5:47:36 PM (12 years ago)

Author:

Simon Fraser

Message:

Kill updateStyleForAllDocuments()
​https://bugs.webkit.org/show_bug.cgi?id=118093

Reviewed by Geoff Garen.

We should never have to eagerly update style, especially for all documents
that the process knows about. The WebKit model is to update style when
required for layout etc, not to update it eagerly.

This is historical code whose origins are lost in the mists of KHTML.
r42384 makes it possible to do away with this eager updating, and
r55568 removed some calls. Discussion in bug 32580 suggests that
form control code still need this, but tests still pass without it.

• bindings/js/JSCallbackData.cpp:

(WebCore::JSCallbackData::invokeCallback):

• bindings/js/JSCustomXPathNSResolver.cpp:

(WebCore::JSCustomXPathNSResolver::lookupNamespaceURI):

• dom/Document.cpp:

(WebCore::Document::scheduleStyleRecalc):
(WebCore::Document::unscheduleStyleRecalc):

• dom/Document.h:
• dom/Element.cpp:

(WebCore::Element::setActive):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• bindings/js/JSCallbackData.cpp (modified) (1 diff)
• bindings/js/JSCustomXPathNSResolver.cpp (modified) (1 diff)
• dom/Document.cpp (modified) (4 diffs)
• dom/Document.h (modified) (1 diff)
• dom/Element.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-08-08  Simon Fraser  <simon.fraser@apple.com>
+
+        Kill updateStyleForAllDocuments()
+        https://bugs.webkit.org/show_bug.cgi?id=118093
+
+        Reviewed by Geoff Garen.
+
+        We should never have to eagerly update style, especially for all documents
+        that the process knows about. The WebKit model is to update style when
+        required for layout etc, not to update it eagerly.
+        
+        This is historical code whose origins are lost in the mists of KHTML.
+        r42384 makes it possible to do away with this eager updating, and
+        r55568 removed some calls. Discussion in bug 32580 suggests that
+        form control code still need this, but tests still pass without it.
+
+        * bindings/js/JSCallbackData.cpp:
+        (WebCore::JSCallbackData::invokeCallback):
+        * bindings/js/JSCustomXPathNSResolver.cpp:
+        (WebCore::JSCustomXPathNSResolver::lookupNamespaceURI):
+        * dom/Document.cpp:
+        (WebCore::Document::scheduleStyleRecalc):
+        (WebCore::Document::unscheduleStyleRecalc):
+        * dom/Document.h:
+        * dom/Element.cpp:
+        (WebCore::Element::setActive):
+
 2013-08-08  Jer Noble  <jer.noble@apple.com>
 

trunk/Source/WebCore/bindings/js/JSCallbackData.cpp

@@ -73 +73 @@
     InspectorInstrumentationCookie cookie = JSMainThreadExecState::instrumentFunctionCall(context, callType, callData);
 
-    bool contextIsDocument = context->isDocument();
-    JSValue result = contextIsDocument
+    JSValue result = context->isDocument()
         ? JSMainThreadExecState::call(exec, function, callType, callData, thisValue, args)
         : JSC::call(exec, function, callType, callData, thisValue, args);
 
     InspectorInstrumentation::didCallFunction(cookie);
-
-    if (contextIsDocument)
-        Document::updateStyleForAllDocuments();
 
     if (exec->hadException()) {

trunk/Source/WebCore/bindings/js/JSCustomXPathNSResolver.cpp

@@ -102 +102 @@
     }
 
-    Document::updateStyleForAllDocuments();
-
     return result;
 }

trunk/Source/WebCore/dom/Document.cpp

@@ -380 +380 @@
 }
 
-static HashSet<Document*>* documentsThatNeedStyleRecalc = 0;
-
 uint64_t Document::s_globalTreeVersion = 0;
 
@@ -1701 +1699 @@
     ASSERT(childNeedsStyleRecalc() || m_pendingStyleRecalcShouldForce);
 
-    if (!documentsThatNeedStyleRecalc)
-        documentsThatNeedStyleRecalc = new HashSet<Document*>;
-    documentsThatNeedStyleRecalc->add(this);
-    
     // FIXME: Why on earth is this here? This is clearly misplaced.
     invalidateAccessKeyMap();
@@ -1716 +1710 @@
 {
     ASSERT(!childNeedsStyleRecalc());
-
-    if (documentsThatNeedStyleRecalc)
-        documentsThatNeedStyleRecalc->remove(this);
 
     m_styleRecalcTimer.stop();
@@ -1855 +1846 @@
     AnimationUpdateBlock animationUpdateBlock(m_frame ? m_frame->animation() : 0);
     recalcStyle(Style::NoChange);
-}
-
-void Document::updateStyleForAllDocuments()
-{
-    ASSERT(isMainThread());
-    if (!documentsThatNeedStyleRecalc)
-        return;
-
-    while (documentsThatNeedStyleRecalc->size()) {
-        HashSet<Document*>::iterator it = documentsThatNeedStyleRecalc->begin();
-        Document* doc = *it;
-        documentsThatNeedStyleRecalc->remove(doc);
-        doc->updateStyleIfNeeded();
-    }
 }
 

trunk/Source/WebCore/dom/Document.h

@@ -526 +526 @@
     void pageSizeAndMarginsInPixels(int pageIndex, IntSize& pageSize, int& marginTop, int& marginRight, int& marginBottom, int& marginLeft);
 
-    static void updateStyleForAllDocuments(); // FIXME: Try to reduce the # of calls to this function.
     CachedResourceLoader* cachedResourceLoader() { return m_cachedResourceLoader.get(); }
 

trunk/Source/WebCore/dom/Element.cpp

@@ -486 +486 @@
 #endif
 
-        Document::updateStyleForAllDocuments();
+        document()->updateStyleIfNeeded();
+
         // Do an immediate repaint.
         if (renderer())