Attachment 348204 Details for Bug 189007 – Patch

[patch] Patch

bug-189007-20180827175332.patch (text/plain), 12.54 KB, created by Ali Juma on 2018-08-27 14:53:33 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ali Juma

Created: 2018-08-27 14:53:33 PDT

Size: 12.54 KB

patch

obsolete

>Subversion Revision: 235359
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index a30dd6e8e76e39d08f87530e32d368c6a38c5779..c8b30b342e970ef590d7a5c8cb2c7d78777e253d 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2018-08-27  Ali Juma  <ajuma@chromium.org>
>+
>+        [IntersectionObserver] Schedule intersection observation updates
>+        https://bugs.webkit.org/show_bug.cgi?id=189007
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Schedule intersection observation updates in the following situations:
>+        1) A new observation target is added.
>+        2) FrameView::viewportContentsChanged -- this covers changes to layout and
>+           to scroll positions.
>+        3) Style is resolved without triggering layout -- this handles updates that
>+           were deferred because of a pending style recalculation.
>+
>+        Tested by existing tests in imported/w3c/web-platform-tests/intersection-observer.
>+
>+        * dom/Document.cpp:
>+        (WebCore::Document::resolveStyle):
>+        (WebCore::Document::updateIntersectionObservations):
>+        (WebCore::Document::scheduleIntersectionObservationUpdate):
>+        * dom/Document.h:
>+        * page/FrameView.cpp:
>+        (WebCore::FrameView::viewportContentsChanged):
>+        * page/IntersectionObserver.cpp:
>+        (WebCore::IntersectionObserver::observe):
>+
> 2018-08-27  Ali Juma  <ajuma@chromium.org>
> 
>         [IntersectionObserver] Implement intersection logic for the explicit root case
>diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
>index 29ef3fbe8ce9ba5b2bc1093dfcded736dd0f2778..dbe14c73142087963dbf7fe82c63be4097c7fd11 100644
>--- a/Source/WebCore/dom/Document.cpp
>+++ b/Source/WebCore/dom/Document.cpp
>@@ -507,6 +507,7 @@ Document::Document(Frame* frame, const URL& url, unsigned documentClasses, unsig
>     , m_documentClasses(documentClasses)
>     , m_eventQueue(*this)
> #if ENABLE(INTERSECTION_OBSERVER)
>+    , m_intersectionObservationUpdateTimer(*this, &Document::updateIntersectionObservations)
>     , m_intersectionObserversNotifyTimer(*this, &Document::notifyIntersectionObserversTimerFired)
> #endif
>     , m_loadEventDelayTimer(*this, &Document::loadEventDelayTimerFired)
>@@ -1878,8 +1879,11 @@ void Document::resolveStyle(ResolveStyleType type)
>             frameView.layoutContext().scheduleLayout();
> 
>         // Usually this is handled by post-layout.
>-        if (!frameView.needsLayout())
>+        if (!frameView.needsLayout()) {
>             frameView.frame().selection().scheduleAppearanceUpdateAfterStyleChange();
>+            if (m_needsIntersectionObservationUpdate)
>+                scheduleIntersectionObservationUpdate();
>+        }
> 
>         // As a result of the style recalculation, the currently hovered element might have been
>         // detached (for example, by setting display:none in the :hover style), schedule another mouseMove event
>@@ -7488,10 +7492,17 @@ static void computeIntersectionRects(IntersectionObserver& observer, Element& ta
> 
> void Document::updateIntersectionObservations()
> {
>+    ASSERT(m_needsIntersectionObservationUpdate);
>     auto* frameView = view();
>     if (!frameView)
>         return;
> 
>+    bool needsLayout = frameView->layoutContext().isLayoutPending() || (renderView() && renderView()->needsLayout());
>+    if (needsLayout || hasPendingStyleRecalc())
>+        return;
>+
>+    m_needsIntersectionObservationUpdate = false;
>+
>     for (auto observer : m_intersectionObservers) {
>         bool needNotify = false;
>         DOMHighResTimeStamp timestamp;
>@@ -7555,6 +7566,15 @@ void Document::updateIntersectionObservations()
>         m_intersectionObserversNotifyTimer.startOneShot(0_s);
> }
> 
>+void Document::scheduleIntersectionObservationUpdate()
>+{
>+    if (m_intersectionObservers.isEmpty() || m_intersectionObservationUpdateTimer.isActive())
>+        return;
>+
>+    m_needsIntersectionObservationUpdate = true;
>+    m_intersectionObservationUpdateTimer.startOneShot(0_s);
>+}
>+
> void Document::notifyIntersectionObserversTimerFired()
> {
>     for (auto observer : m_intersectionObserversWithPendingNotifications) {
>diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h
>index 0a1d903417d4ca9dc0e30bbffac41d188f31bdbc..0b457bdc1d158c054c8636853462f3438cb35d20 100644
>--- a/Source/WebCore/dom/Document.h
>+++ b/Source/WebCore/dom/Document.h
>@@ -1368,6 +1368,7 @@ public:
>     RefPtr<IntersectionObserver> removeIntersectionObserver(IntersectionObserver&);
>     unsigned numberOfIntersectionObservers() const { return m_intersectionObservers.size(); }
>     void updateIntersectionObservations();
>+    void scheduleIntersectionObservationUpdate();
> #endif
> 
> #if ENABLE(MEDIA_STREAM)
>@@ -1775,7 +1776,10 @@ private:
> #if ENABLE(INTERSECTION_OBSERVER)
>     Vector<RefPtr<IntersectionObserver>> m_intersectionObservers;
>     Vector<WeakPtr<IntersectionObserver>> m_intersectionObserversWithPendingNotifications;
>+    Timer m_intersectionObservationUpdateTimer;
>     Timer m_intersectionObserversNotifyTimer;
>+
>+    bool m_needsIntersectionObservationUpdate { false };
> #endif
> 
>     Timer m_loadEventDelayTimer;
>diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
>index 8df45e94ed51b15f018f8ab590a6df66c7fdd42d..6f1f76c37f402e78386718df7229ae93a9b4f306 100644
>--- a/Source/WebCore/page/FrameView.cpp
>+++ b/Source/WebCore/page/FrameView.cpp
>@@ -2001,6 +2001,8 @@ void FrameView::viewportContentsChanged()
>         if (auto* renderView = frameView.frame().contentRenderer())
>             renderView->updateVisibleViewportRect(visibleRect);
>     });
>+
>+    frame().document()->scheduleIntersectionObservationUpdate();
> }
> 
> bool FrameView::fixedElementsLayoutRelativeToFrame() const
>diff --git a/Source/WebCore/page/IntersectionObserver.cpp b/Source/WebCore/page/IntersectionObserver.cpp
>index 4bbe1926cd810baa755585af217e97d73565ebe6..5e64f7592bc02802f963c1b36b206a79241e6ccf 100644
>--- a/Source/WebCore/page/IntersectionObserver.cpp
>+++ b/Source/WebCore/page/IntersectionObserver.cpp
>@@ -154,9 +154,7 @@ void IntersectionObserver::observe(Element& target)
>     auto* document = trackingDocument();
>     if (!hadObservationTargets)
>         document->addIntersectionObserver(this);
>-    document->postTask([document] (ScriptExecutionContext&) mutable {
>-        document->updateIntersectionObservations();
>-    });
>+    document->scheduleIntersectionObservationUpdate();
> }
> 
> void IntersectionObserver::unobserve(Element& target)
>diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
>index 4cd255245792597f500e1561c061fcbe69bcaf1e..0b5636aa4838061d7dc34425b348d839972af300 100644
>--- a/LayoutTests/imported/w3c/ChangeLog
>+++ b/LayoutTests/imported/w3c/ChangeLog
>@@ -1,3 +1,18 @@
>+2018-08-27  Ali Juma  <ajuma@chromium.org>
>+
>+        [IntersectionObserver] Schedule intersection observation updates
>+        https://bugs.webkit.org/show_bug.cgi?id=189007
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Rebaseline tests that now pass.
>+
>+        * web-platform-tests/intersection-observer/bounding-box-expected.txt:
>+        * web-platform-tests/intersection-observer/containing-block-expected.txt:
>+        * web-platform-tests/intersection-observer/remove-element-expected.txt:
>+        * web-platform-tests/intersection-observer/same-document-root-expected.txt:
>+        * web-platform-tests/intersection-observer/unclipped-root-expected.txt:
>+
> 2018-08-27  Ali Juma  <ajuma@chromium.org>
> 
>         [IntersectionObserver] Implement intersection logic for the explicit root case
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/bounding-box-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/bounding-box-expected.txt
>index 865f24bd1b1587d2941ce4e8b1ec7d8eedd9d231..848114318477a25f5d7c6c609ec56acc93969bb4 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/bounding-box-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/bounding-box-expected.txt
>@@ -1,5 +1,5 @@
> 
> PASS Test that the target's border bounding box is used to calculate intersection. 
> PASS First rAF. 
>-FAIL target.style.transform = 'translateY(195px)' assert_equals: entries.length expected 2 but got 1
>+PASS target.style.transform = 'translateY(195px)' 
> 
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/containing-block-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/containing-block-expected.txt
>index 7499738da3056f5f0cac8eab9e3a987ffdb280bd..0f1f801f3e178bb97409aeddce9561a213ec627c 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/containing-block-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/containing-block-expected.txt
>@@ -1,7 +1,7 @@
> 
> PASS IntersectionObserver should only report intersections if root is a containing block ancestor of target. 
>-FAIL In containing block and intersecting. assert_equals: entries[0].boundingClientRect.top expected 18 but got 258
>-FAIL In containing block and not intersecting. assert_equals: entries.length expected 2 but got 1
>-FAIL Not in containing block and intersecting. assert_equals: entries.length expected 2 but got 1
>-FAIL Not in containing block and not intersecting. assert_equals: entries.length expected 2 but got 1
>+PASS In containing block and intersecting. 
>+PASS In containing block and not intersecting. 
>+PASS Not in containing block and intersecting. 
>+PASS Not in containing block and not intersecting. 
> 
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/remove-element-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/remove-element-expected.txt
>index 9f9575dce60c263ca4a7aec4399e0cb0459acde2..e51f12bd127a6698eb158b19cb9095d1d7b233a4 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/remove-element-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/remove-element-expected.txt
>@@ -1,8 +1,8 @@
> 
> PASS Verify that not-intersecting notifications are sent when a target is removed from the DOM tree. 
> PASS First rAF 
>-FAIL root.scrollTop = 150 assert_equals: entries.length expected 2 but got 1
>-FAIL root.removeChild(target). assert_equals: entries.length expected 3 but got 1
>-FAIL root.insertBefore(target, trailingSpace). assert_equals: entries.length expected 3 but got 1
>-FAIL root.scrollTop = 150 after reinserting target. assert_equals: entries.length expected 4 but got 1
>+PASS root.scrollTop = 150 
>+PASS root.removeChild(target). 
>+PASS root.insertBefore(target, trailingSpace). 
>+PASS root.scrollTop = 150 after reinserting target. 
> 
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/same-document-root-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/same-document-root-expected.txt
>index 35ccde794bdc28c114128b16377273bae8287bed..9e1fc864dc910f042c3ea05fb01a8affcd6bf9ef 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/same-document-root-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/same-document-root-expected.txt
>@@ -2,8 +2,8 @@
> PASS IntersectionObserver in a single document with explicit root. 
> PASS First rAF 
> PASS document.scrollingElement.scrollTop = window.innerHeight. 
>-FAIL root.scrollTop = 150 with root scrolled into view. assert_equals: entries.length expected 2 but got 1
>-FAIL document.scrollingElement.scrollTop = 0. assert_equals: entries.length expected 2 but got 1
>-FAIL root.scrollTop = 0 assert_equals: entries.length expected 3 but got 1
>-FAIL root.scrollTop = 150 with root scrolled out of view. assert_equals: entries.length expected 4 but got 1
>+PASS root.scrollTop = 150 with root scrolled into view. 
>+PASS document.scrollingElement.scrollTop = 0. 
>+PASS root.scrollTop = 0 
>+PASS root.scrollTop = 150 with root scrolled out of view. 
> 
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/unclipped-root-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/unclipped-root-expected.txt
>index 75223f295cfd98230a2e3268f2277f03fd6dce41..4729502374358176a7873cbb4a81686d02b5ed86 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/unclipped-root-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/unclipped-root-expected.txt
>@@ -1,5 +1,5 @@
> 
> PASS Test that border bounding box is used to calculate intersection with a non-scrolling root. 
> PASS First rAF. 
>-FAIL target.style.transform = 'translateY(195px)' assert_equals: entries.length expected 2 but got 1
>+PASS target.style.transform = 'translateY(195px)' 
>

Actions: View | Formatted Diff | Diff

Attachments on bug 189007: 348204 | 348236 | 348237 | 348290