Attachment 346988 Details for Bug 188491 – Patch

[patch] Patch

bug-188491-20180813055734.patch (text/plain), 12.07 KB, created by Yusuke Suzuki on 2018-08-12 13:57:35 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-08-12 13:57:35 PDT

Size: 12.07 KB

patch

obsolete

>Subversion Revision: 234787
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 3aee6be28631e84216016f833dc72dfc3e4f30ab..a31e25e5e69e0c089c0f1ffb28464b917d1543ef 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,34 @@
>+2018-08-12  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
>+
>+        ErrorEvent should not hold error strongly
>+        https://bugs.webkit.org/show_bug.cgi?id=188491
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        ErrorEvent should not use Strong<Unkonwn> to hold error JSValue. This patch integrates
>+        JSValueInWrappedObject into ErrorEvent.
>+
>+        * Modules/webvr/VRDisplayEvent.h:
>+        Fix unified build errors due to added JSErrorEventCustom.cpp. It changes the files grouped in unified build.
>+
>+        * Sources.txt:
>+        * bindings/js/JSErrorEventCustom.cpp: Copied from Source/WebCore/Modules/webvr/VRDisplayEvent.h.
>+        (WebCore::JSErrorEvent::visitAdditionalChildren):
>+        Add custom mark function for JSValueInWrappedObject.
>+
>+        * bindings/js/JSEventListener.h:
>+        * bindings/js/WindowProxy.cpp:
>+        Fix unified build errors due to added JSErrorEventCustom.cpp. It changes the files grouped in unified build.
>+
>+        * dom/ErrorEvent.cpp:
>+        (WebCore::ErrorEvent::ErrorEvent):
>+        (WebCore::ErrorEvent::error):
>+        (WebCore::ErrorEvent::trySerializeError):
>+        Align the implementation to PushStateEvent::trySerializeState.
>+
>+        * dom/ErrorEvent.h:
>+        * dom/ErrorEvent.idl:
>+
> 2018-08-12  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] Float prev/next sibling should prevent top/bottom margin collapsing with parent.
>diff --git a/Source/WebCore/Modules/webvr/VRDisplayEvent.h b/Source/WebCore/Modules/webvr/VRDisplayEvent.h
>index 9e19741a311d79b86a93d97cb741bcc03a21c0bd..dae7a6d1d25d51282ddebb62b26e5a6040ecce3a 100644
>--- a/Source/WebCore/Modules/webvr/VRDisplayEvent.h
>+++ b/Source/WebCore/Modules/webvr/VRDisplayEvent.h
>@@ -25,12 +25,11 @@
> #pragma once
> 
> #include "Event.h"
>+#include "VRDisplay.h"
> #include "VRDisplayEventReason.h"
> 
> namespace WebCore {
> 
>-class VRDisplay;
>-
> class VRDisplayEvent final : public Event {
> public:
>     static Ref<VRDisplayEvent> create(const AtomicString& type, const RefPtr<VRDisplay>& display, std::optional<VRDisplayEventReason>&& reason)
>diff --git a/Source/WebCore/Sources.txt b/Source/WebCore/Sources.txt
>index 379275fff22049211764641d4374c554bff17843..70a01639b46333b558cc23ae3694aa7c3439fa52 100644
>--- a/Source/WebCore/Sources.txt
>+++ b/Source/WebCore/Sources.txt
>@@ -397,6 +397,7 @@ bindings/js/JSDOMWrapperCache.cpp
> bindings/js/JSDocumentCustom.cpp
> bindings/js/JSDocumentFragmentCustom.cpp
> bindings/js/JSElementCustom.cpp
>+bindings/js/JSErrorEventCustom.cpp
> bindings/js/JSErrorHandler.cpp
> bindings/js/JSEventCustom.cpp
> bindings/js/JSEventListener.cpp
>diff --git a/Source/WebCore/bindings/js/JSErrorEventCustom.cpp b/Source/WebCore/bindings/js/JSErrorEventCustom.cpp
>new file mode 100644
>index 0000000000000000000000000000000000000000..1b0e4498315cc86cd10ee2973cc4de99a8beb649
>--- /dev/null
>+++ b/Source/WebCore/bindings/js/JSErrorEventCustom.cpp
>@@ -0,0 +1,36 @@
>+/*
>+ * Copyright (C) 2018 Yusuke Suzuki <yusukesuzuki@slowstart.org>.
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ * 1. Redistributions of source code must retain the above copyright
>+ *    notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ *    notice, this list of conditions and the following disclaimer in the
>+ *    documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
>+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
>+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
>+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
>+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
>+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
>+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
>+ * THE POSSIBILITY OF SUCH DAMAGE.
>+ */
>+
>+#include "config.h"
>+#include "JSErrorEvent.h"
>+
>+namespace WebCore {
>+
>+void JSErrorEvent::visitAdditionalChildren(JSC::SlotVisitor& visitor)
>+{
>+    wrapped().originalError().visit(visitor);
>+}
>+
>+} // namespace WebCore
>diff --git a/Source/WebCore/bindings/js/JSEventListener.h b/Source/WebCore/bindings/js/JSEventListener.h
>index eccf937c6fb5db3c85e2b05d384b297a33d6ab06..cb8880baee309f03e419ba70326859cf06225f60 100644
>--- a/Source/WebCore/bindings/js/JSEventListener.h
>+++ b/Source/WebCore/bindings/js/JSEventListener.h
>@@ -32,6 +32,7 @@
> namespace WebCore {
> 
> class DOMWindow;
>+class Document;
> class EventTarget;
> class HTMLElement;
> 
>diff --git a/Source/WebCore/bindings/js/WindowProxy.cpp b/Source/WebCore/bindings/js/WindowProxy.cpp
>index dcf1e0e560ca9c9a75fa58a3da9d902cfc982d1a..91b0d6dc9ec0519b554b49bfd6bebf8425913b24 100644
>--- a/Source/WebCore/bindings/js/WindowProxy.cpp
>+++ b/Source/WebCore/bindings/js/WindowProxy.cpp
>@@ -36,6 +36,8 @@
> 
> namespace WebCore {
> 
>+using namespace JSC;
>+
> static void collectGarbageAfterWindowProxyDestruction()
> {
>     // Make sure to GC Extra Soon(tm) during memory pressure conditions
>diff --git a/Source/WebCore/dom/ErrorEvent.cpp b/Source/WebCore/dom/ErrorEvent.cpp
>index 2f4a4e537154ed7e05ee836ccfec22ddcb239a78..d9dbbdcd758634c4e199d79e486ad0515077d666 100644
>--- a/Source/WebCore/dom/ErrorEvent.cpp
>+++ b/Source/WebCore/dom/ErrorEvent.cpp
>@@ -40,13 +40,13 @@
> namespace WebCore {
> using namespace JSC;
> 
>-ErrorEvent::ErrorEvent(ExecState& state, const AtomicString& type, const Init& initializer, IsTrusted isTrusted)
>+ErrorEvent::ErrorEvent(const AtomicString& type, const Init& initializer, IsTrusted isTrusted)
>     : Event(type, initializer, isTrusted)
>     , m_message(initializer.message)
>     , m_fileName(initializer.filename)
>     , m_lineNumber(initializer.lineno)
>     , m_columnNumber(initializer.colno)
>-    , m_error(state.vm(), initializer.error)
>+    , m_error(initializer.error)
> {
> }
> 
>@@ -56,7 +56,7 @@ ErrorEvent::ErrorEvent(const String& message, const String& fileName, unsigned l
>     , m_fileName(fileName)
>     , m_lineNumber(lineNumber)
>     , m_columnNumber(columnNumber)
>-    , m_error(error)
>+    , m_error(error.get())
> {
> }
> 
>@@ -69,7 +69,7 @@ EventInterface ErrorEvent::eventInterface() const
> 
> JSValue ErrorEvent::error(ExecState& state, JSGlobalObject& globalObject)
> {    
>-    auto error = m_error.get();
>+    JSValue error = m_error;
>     if (!error)
>         return jsNull();
> 
>@@ -87,11 +87,11 @@ JSValue ErrorEvent::error(ExecState& state, JSGlobalObject& globalObject)
> 
> RefPtr<SerializedScriptValue> ErrorEvent::trySerializeError(ExecState& exec)
> {
>-    if (!m_triedToSerialize) {
>-        m_serializedDetail = SerializedScriptValue::create(exec, m_error.get(), SerializationErrorMode::NonThrowing);
>+    if (!m_serializedError && !m_triedToSerialize) {
>+        m_serializedError = SerializedScriptValue::create(exec, m_error, SerializationErrorMode::NonThrowing);
>         m_triedToSerialize = true;
>     }
>-    return m_serializedDetail;
>+    return m_serializedError;
> }
> 
> bool ErrorEvent::isErrorEvent() const
>diff --git a/Source/WebCore/dom/ErrorEvent.h b/Source/WebCore/dom/ErrorEvent.h
>index 3f37500b0709b5db14f114c2b3ea795fbd16f990..a85e93cca661ab7f4f4063bb1a737c6fa1187c5d 100644
>--- a/Source/WebCore/dom/ErrorEvent.h
>+++ b/Source/WebCore/dom/ErrorEvent.h
>@@ -32,6 +32,7 @@
> #pragma once
> 
> #include "Event.h"
>+#include "JSValueInWrappedObject.h"
> #include "SerializedScriptValue.h"
> #include <JavaScriptCore/Strong.h>
> #include <wtf/text/WTFString.h>
>@@ -53,9 +54,9 @@ class ErrorEvent final : public Event {
>         JSC::JSValue error;
>     };
> 
>-    static Ref<ErrorEvent> create(JSC::ExecState& state, const AtomicString& type, const Init& initializer, IsTrusted isTrusted = IsTrusted::No)
>+    static Ref<ErrorEvent> create(const AtomicString& type, const Init& initializer, IsTrusted isTrusted = IsTrusted::No)
>     {
>-        return adoptRef(*new ErrorEvent(state, type, initializer, isTrusted));
>+        return adoptRef(*new ErrorEvent(type, initializer, isTrusted));
>     }
> 
>     virtual ~ErrorEvent();
>@@ -66,13 +67,16 @@ class ErrorEvent final : public Event {
>     unsigned colno() const { return m_columnNumber; }
>     JSC::JSValue error(JSC::ExecState&, JSC::JSGlobalObject&);
> 
>+    const JSValueInWrappedObject& originalError() const { return m_error; }
>+    SerializedScriptValue* serializedError() const { return m_serializedError.get(); }
>+
>     EventInterface eventInterface() const override;
> 
>+    RefPtr<SerializedScriptValue> trySerializeError(JSC::ExecState&);
>+
> private:
>     ErrorEvent(const String& message, const String& fileName, unsigned lineNumber, unsigned columnNumber, JSC::Strong<JSC::Unknown> error);
>-    ErrorEvent(JSC::ExecState&, const AtomicString&, const Init&, IsTrusted);
>-
>-    RefPtr<SerializedScriptValue> trySerializeError(JSC::ExecState&);
>+    ErrorEvent(const AtomicString&, const Init&, IsTrusted);
> 
>     bool isErrorEvent() const override;
> 
>@@ -80,10 +84,8 @@ class ErrorEvent final : public Event {
>     String m_fileName;
>     unsigned m_lineNumber;
>     unsigned m_columnNumber;
>-    // FIXME: The following use of JSC::Strong is incorrect and can lead to storage leaks
>-    // due to reference cycles; we should use JSValueInWrappedObject instead.
>-    JSC::Strong<JSC::Unknown> m_error;
>-    RefPtr<SerializedScriptValue> m_serializedDetail;
>+    JSValueInWrappedObject m_error;
>+    RefPtr<SerializedScriptValue> m_serializedError;
>     bool m_triedToSerialize { false };
> };
> 
>diff --git a/Source/WebCore/dom/ErrorEvent.idl b/Source/WebCore/dom/ErrorEvent.idl
>index edb5e4b8704fd9f25684cd5572768bdfd6c635c7..517a07c3b0665d863898f3239326e243f5f9ffee 100644
>--- a/Source/WebCore/dom/ErrorEvent.idl
>+++ b/Source/WebCore/dom/ErrorEvent.idl
>@@ -31,8 +31,8 @@
> 
> [
>     Constructor(DOMString type, optional ErrorEventInit eventInitDict),
>-    ConstructorCallWith=ScriptState,
>     Exposed=(Window,Worker),
>+    JSCustomMarkFunction,
> ] interface ErrorEvent : Event {
>     readonly attribute DOMString message;
>     readonly attribute USVString filename;
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 932161c1d720407e498e8055d8b75aa0c07eeb6d..0082b09d8fb1e96040e824f546bfd0025e091227 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,12 @@
>+2018-08-12  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
>+
>+        ErrorEvent should not hold error strongly
>+        https://bugs.webkit.org/show_bug.cgi?id=188491
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/dom/reference-cycle-leaks-expected.txt:
>+
> 2018-08-12  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] Float prev/next sibling should prevent top/bottom margin collapsing with parent.
>diff --git a/LayoutTests/fast/dom/reference-cycle-leaks-expected.txt b/LayoutTests/fast/dom/reference-cycle-leaks-expected.txt
>index 461954625636b76dc64796961b125c3cc1624398..aa337a075e2b88707cc3aa1608cfdbc6f14ffc5a 100644
>--- a/LayoutTests/fast/dom/reference-cycle-leaks-expected.txt
>+++ b/LayoutTests/fast/dom/reference-cycle-leaks-expected.txt
>@@ -10,7 +10,7 @@ PASS checkForNodeLeaks(createTreeWalkerNodeCycle) is "did not leak"
> PASS checkForNodeLeaks(createTreeWalkerFilterCycle) is "did not leak"
> PASS checkForNodeLeaks(createPromiseCycle) is "did not leak"
> PASS checkForNodeLeaks(createCustomEventDetailsCycle) is "did not leak"
>-FAIL checkForNodeLeaks(createErrorEventDataCycle) should be did not leak. Was leaked.
>+PASS checkForNodeLeaks(createErrorEventDataCycle) is "did not leak"
> ---- Did not test ExtendableMessageEvent because it is not enabled.
> PASS checkForNodeLeaks(createMessageEventDataCycle) is "did not leak"
> PASS checkForNodeLeaks(createPopStateEventStateCycle) is "did not leak"

Actions: View | Formatted Diff | Diff

Attachments on bug 188491: 346988 | 346989