Changeset 243305 in webkit

Timestamp:

Mar 21, 2019, 11:38:39 AM (7 years ago)

Author:

Alan Bujtas

Message:

[ContentChangeObserver] Track hidden elements only while transitioning.
​https://bugs.webkit.org/show_bug.cgi?id=196050
<rdar://problem/49092037>

Reviewed by Simon Fraser.

Use the existing isConsideredHidden() logic to decide whether the current transition should be tracked.

• page/ios/ContentChangeObserver.cpp:

(WebCore::isConsideredHidden):
(WebCore::ContentChangeObserver::didAddTransition):
(WebCore::ContentChangeObserver::StyleChangeScope::StyleChangeScope):
(WebCore::ContentChangeObserver::StyleChangeScope::~StyleChangeScope):
(WebCore::ContentChangeObserver::StyleChangeScope::isConsideredHidden const): Deleted.

• page/ios/ContentChangeObserver.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• page/ios/ContentChangeObserver.cpp (modified) (4 diffs)
• page/ios/ContentChangeObserver.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-03-21  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] Track hidden elements only while transitioning.
+        https://bugs.webkit.org/show_bug.cgi?id=196050
+        <rdar://problem/49092037>
+
+        Reviewed by Simon Fraser.
+
+        Use the existing isConsideredHidden() logic to decide whether the current transition should be tracked.
+
+        * page/ios/ContentChangeObserver.cpp:
+        (WebCore::isConsideredHidden):
+        (WebCore::ContentChangeObserver::didAddTransition):
+        (WebCore::ContentChangeObserver::StyleChangeScope::StyleChangeScope):
+        (WebCore::ContentChangeObserver::StyleChangeScope::~StyleChangeScope):
+        (WebCore::ContentChangeObserver::StyleChangeScope::isConsideredHidden const): Deleted.
+        * page/ios/ContentChangeObserver.h:
+
 2019-03-21  Zalan Bujtas  <zalan@apple.com>
 

trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp

@@ -43 +43 @@
 static const Seconds maximumDelayForTransitions { 300_ms };
 
+static bool isConsideredHidden(const Element& element)
+{
+    if (!element.renderStyle())
+        return true;
+
+    auto& style = *element.renderStyle();
+    if (style.display() == DisplayType::None)
+        return true;
+
+    if (style.visibility() == Visibility::Hidden)
+        return true;
+
+    auto width = style.logicalWidth();
+    auto height = style.logicalHeight();
+    if ((width.isFixed() && !width.value()) || (height.isFixed() && !height.value()))
+        return true;
+
+    auto top = style.logicalTop();
+    auto left = style.logicalLeft();
+    // FIXME: This is trying to check if the element is outside of the viewport. This is incorrect for many reasons.
+    if (left.isFixed() && width.isFixed() && -left.value() >= width.value())
+        return true;
+    if (top.isFixed() && height.isFixed() && -top.value() >= height.value())
+        return true;
+
+    // It's a common technique used to position content offscreen.
+    if (style.hasOutOfFlowPosition() && left.isFixed() && left.value() <= -999)
+        return true;
+
+    // FIXME: Check for other cases like zero height with overflow hidden.
+    auto maxHeight = style.maxHeight();
+    if (maxHeight.isFixed() && !maxHeight.value())
+        return true;
+
+    return false;
+}
+
 ContentChangeObserver::ContentChangeObserver(Document& document)
     : m_document(document)
@@ -99 +136 @@
     auto transitionEnd = Seconds { transition.duration() + std::max<double>(0, transition.isDelaySet() ? transition.delay() : 0) };
     if (transitionEnd > maximumDelayForTransitions)
+        return;
+    if (!isConsideredHidden(element))
         return;
     LOG_WITH_STREAM(ContentObservation, stream << "didAddTransition: transition created on " << &element << " (" << transitionEnd.milliseconds() << "ms).");
@@ -385 +424 @@
 {
     if (m_contentChangeObserver.isObservingContentChanges() && !m_contentChangeObserver.hasVisibleChangeState())
-        m_wasHidden = isConsideredHidden();
+        m_wasHidden = isConsideredHidden(m_element);
 }
 
@@ -391 +430 @@
 {
     auto changedFromHiddenToVisible = [&] {
-        return m_wasHidden && !isConsideredHidden();
+        return m_wasHidden && !isConsideredHidden(m_element);
     };
 
     if (changedFromHiddenToVisible() && isConsideredClickable())
         m_contentChangeObserver.contentVisibilityDidChange();
-}
-
-bool ContentChangeObserver::StyleChangeScope::isConsideredHidden() const
-{
-    if (!m_element.renderStyle())
-        return true;
-
-    auto& style = *m_element.renderStyle();
-    if (style.display() == DisplayType::None)
-        return true;
-
-    if (style.visibility() == Visibility::Hidden)
-        return true;
-
-    auto width = style.logicalWidth();
-    auto height = style.logicalHeight();
-    if ((width.isFixed() && !width.value()) || (height.isFixed() && !height.value()))
-        return true;
-
-    auto top = style.logicalTop();
-    auto left = style.logicalLeft();
-    // FIXME: This is trying to check if the element is outside of the viewport. This is incorrect for many reasons.
-    if (left.isFixed() && width.isFixed() && -left.value() >= width.value())
-        return true;
-    if (top.isFixed() && height.isFixed() && -top.value() >= height.value())
-        return true;
-
-    // It's a common technique used to position content offscreen.
-    if (style.hasOutOfFlowPosition() && left.isFixed() && left.value() <= -999)
-        return true;
-
-    // FIXME: Check for other cases like zero height with overflow hidden.
-    auto maxHeight = style.maxHeight();
-    if (maxHeight.isFixed() && !maxHeight.value())
-        return true;
-
-    return false;
 }
 

trunk/Source/WebCore/page/ios/ContentChangeObserver.h

@@ -67 +67 @@
 
     private:
-        bool isConsideredHidden() const;
         bool isConsideredClickable() const;