Invalidate first line display item clients when first line style changes

third_party/WebKit/Source/core/dom/Element.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Peter Kelly (pmk@post.com)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  *           (C) 2007 David Smith (catfish.man@gmail.com)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2012, 2013 Apple Inc. All rights reserved.
  *           (C) 2007 Eric Seidel (eric@webkit.org)
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 1773 matching lines @@
             return Reattach;
         return ReattachNoLayoutObject;
     }
 
     ASSERT(oldStyle);
 
     if (localChange != NoChange)
         updateCallbackSelectors(oldStyle.get(), newStyle.get());
 
     if (LayoutObject* layoutObject = this->layoutObject()) {
-        if (localChange != NoChange || pseudoStyleCacheIsInvalid(oldStyle.get(), newStyle.get()) || svgFilterNeedsLayerUpdate()) {
+        bool pseudoStyleCacheIsInvalid = this->pseudoStyleCacheIsInvalid(oldStyle.get(), newStyle.get());
+        if (localChange != NoChange || pseudoStyleCacheIsInvalid || svgFilterNeedsLayerUpdate()) {

[rune, 2015/12/01 11:09:37]

I'm a bit confused about the role of diffPseudoStyles(), which affects
localChange here, and pseudoStyleCacheIsInvalid(). They seem to do similar
things?

I guess this is the reason for the increased invalidation numbers in the
scrollbar tests?

I suspect this change also fixes crbug.com/563943 which I just reported.

Another thing - the early returns from the for-loop in
pseudoStyleCacheIsInvalid(). If we have both first-line and a different pseudo
element, and we encounter the other pseudo element first in the for-loop, it
seems firstLineStyleDidChange() will not be called because of the early return.
We can handle that in another CL.

[Xianzhu, 2015/12/01 20:06:25]

I see diffPseudoStyles() compares cached pseudo styles, but
pseudoStyleCacheIsInvalid() compares old cached pseudo style and new unchached
pseudo style. I'm not sure when a new style will have cached pseudo style, but
the two functions seem to do different things.
The numbers in the test is the numbers of calls to
StyleResolver::styleForElement(), pseudoStyleForElement() and styleForPage().
There are one styleForElement() call for the element's own style and one
pseudoStyleForElement call for each pseudo style.

diffPseudoStyles() doesn't increase the number because it accesses cached pseudo
style only.
Thanks for reporting the bug. Unfortunately this CL doesn't fix the bug. Will
investigate later.
Done in this CL.

[Xianzhu, 2015/12/01 20:19:09]

I reverted the changes about early returns in the latest patch set. It was
incorrect and further change for crbug.com/563943 would need to change that
again.

             layoutObject->setStyle(newStyle.get());
         } else {
             // Although no change occurred, we use the new style so that the cousin style sharing code won't get
             // fooled into believing this style is the same.
             // FIXME: We may be able to remove this hack, see discussion in
             // https://codereview.chromium.org/30453002/
             layoutObject->setStyleInternal(newStyle.get());
         }
     }
 
@@ skipping 1762 matching lines @@
 {
 #if ENABLE(OILPAN)
     if (hasRareData())
         visitor->trace(elementRareData());
     visitor->trace(m_elementData);
 #endif
     ContainerNode::trace(visitor);
 }
 
 } // namespace blink