Title: [281188] trunk/Source/WebCore
- Revision
- 281188
- Author
- cdu...@apple.com
- Date
- 2021-08-18 08:25:04 -0700 (Wed, 18 Aug 2021)
Log Message
Crash under JSIntersectionObserverCallback::handleEvent()
https://bugs.webkit.org/show_bug.cgi?id=229196
<rdar://82016054>
Reviewed by Geoffrey Garen.
Early return in IntersectionObserver::notify() if the callback has already been destroyed.
This is not supposed to happen as we're supposed to be keeping the JSIntersectionObserver
wrapper alive as long as the IntersectionObserver may fire events, which should keep the
callback alive too. However, despite Ryosuke's fix in r280549, the crash is still happening
in the wild. To address the crash, I am simply doing an early return for now, similarly to
what we already do in MutationObserver::deliver(), while keeping a debug assertion around
in hope of finding the root cause at some point.
No new tests, we do not not how this is happening yet.
* page/IntersectionObserver.cpp:
(WebCore::IntersectionObserver::notify):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (281187 => 281188)
--- trunk/Source/WebCore/ChangeLog 2021-08-18 15:10:54 UTC (rev 281187)
+++ trunk/Source/WebCore/ChangeLog 2021-08-18 15:25:04 UTC (rev 281188)
@@ -1,3 +1,24 @@
+2021-08-18 Chris Dumez <cdu...@apple.com>
+
+ Crash under JSIntersectionObserverCallback::handleEvent()
+ https://bugs.webkit.org/show_bug.cgi?id=229196
+ <rdar://82016054>
+
+ Reviewed by Geoffrey Garen.
+
+ Early return in IntersectionObserver::notify() if the callback has already been destroyed.
+ This is not supposed to happen as we're supposed to be keeping the JSIntersectionObserver
+ wrapper alive as long as the IntersectionObserver may fire events, which should keep the
+ callback alive too. However, despite Ryosuke's fix in r280549, the crash is still happening
+ in the wild. To address the crash, I am simply doing an early return for now, similarly to
+ what we already do in MutationObserver::deliver(), while keeping a debug assertion around
+ in hope of finding the root cause at some point.
+
+ No new tests, we do not not how this is happening yet.
+
+ * page/IntersectionObserver.cpp:
+ (WebCore::IntersectionObserver::notify):
+
2021-08-18 Martin Robinson <mrobin...@webkit.org>
position: sticky with display: inline-block
Modified: trunk/Source/WebCore/html/LazyLoadFrameObserver.cpp (281187 => 281188)
--- trunk/Source/WebCore/html/LazyLoadFrameObserver.cpp 2021-08-18 15:10:54 UTC (rev 281187)
+++ trunk/Source/WebCore/html/LazyLoadFrameObserver.cpp 2021-08-18 15:25:04 UTC (rev 281188)
@@ -43,6 +43,8 @@
}
private:
+ bool hasCallback() const final { return true; }
+
CallbackResult<void> handleEvent(IntersectionObserver&, const Vector<Ref<IntersectionObserverEntry>>& entries, IntersectionObserver&) final
{
ASSERT(!entries.isEmpty());
Modified: trunk/Source/WebCore/html/LazyLoadImageObserver.cpp (281187 => 281188)
--- trunk/Source/WebCore/html/LazyLoadImageObserver.cpp 2021-08-18 15:10:54 UTC (rev 281187)
+++ trunk/Source/WebCore/html/LazyLoadImageObserver.cpp 2021-08-18 15:25:04 UTC (rev 281188)
@@ -43,6 +43,8 @@
}
private:
+ bool hasCallback() const final { return true; }
+
CallbackResult<void> handleEvent(IntersectionObserver&, const Vector<Ref<IntersectionObserverEntry>>& entries, IntersectionObserver&) final
{
ASSERT(!entries.isEmpty());
Modified: trunk/Source/WebCore/page/IntersectionObserver.cpp (281187 => 281188)
--- trunk/Source/WebCore/page/IntersectionObserver.cpp 2021-08-18 15:10:54 UTC (rev 281187)
+++ trunk/Source/WebCore/page/IntersectionObserver.cpp 2021-08-18 15:25:04 UTC (rev 281188)
@@ -273,12 +273,17 @@
return;
}
+ auto takenRecords = takeRecords();
+
+ // FIXME: The JSIntersectionObserver wrapper should be kept alive as long as the intersection observer can fire events.
+ ASSERT(m_callback->hasCallback());
+ if (!m_callback->hasCallback())
+ return;
+
auto* context = m_callback->scriptExecutionContext();
if (!context)
return;
- auto takenRecords = takeRecords();
-
InspectorInstrumentation::willFireObserverCallback(*context, "IntersectionObserver"_s);
m_callback->handleEvent(*this, WTFMove(takenRecords.records), *this);
InspectorInstrumentation::didFireObserverCallback(*context);
Modified: trunk/Source/WebCore/page/IntersectionObserverCallback.h (281187 => 281188)
--- trunk/Source/WebCore/page/IntersectionObserverCallback.h 2021-08-18 15:10:54 UTC (rev 281187)
+++ trunk/Source/WebCore/page/IntersectionObserverCallback.h 2021-08-18 15:25:04 UTC (rev 281188)
@@ -41,7 +41,7 @@
public:
using ActiveDOMCallback::ActiveDOMCallback;
- virtual bool hasCallback() const { return false; }
+ virtual bool hasCallback() const = 0;
virtual CallbackResult<void> handleEvent(IntersectionObserver&, const Vector<Ref<IntersectionObserverEntry>>&, IntersectionObserver&) = 0;
};
Modified: trunk/Source/WebCore/page/ResizeObserver.cpp (281187 => 281188)
--- trunk/Source/WebCore/page/ResizeObserver.cpp 2021-08-18 15:10:54 UTC (rev 281187)
+++ trunk/Source/WebCore/page/ResizeObserver.cpp 2021-08-18 15:25:04 UTC (rev 281188)
@@ -124,6 +124,11 @@
m_activeObservations.clear();
auto activeObservationTargets = std::exchange(m_activeObservationTargets, { });
+ // FIXME: The JSResizeObserver wrapper should be kept alive as long as the resize observer can fire events.
+ ASSERT(m_callback->hasCallback());
+ if (!m_callback->hasCallback())
+ return;
+
auto* context = m_callback->scriptExecutionContext();
if (!context)
return;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
