Attachment 356763 Details for Bug 192480 – Patch

[patch] Patch

bug-192480-20181206163143.patch (text/plain), 13.87 KB, created by Justin Michaud on 2018-12-06 16:31:45 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Justin Michaud

Created: 2018-12-06 16:31:45 PST

Size: 13.87 KB

patch

obsolete

>Subversion Revision: 238933
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index ecf198d652f45c3e24562f08c6a025a49def0788..df1948d9467e458a0455aa1d9064b5018eb6dd93 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2018-12-06  Justin Michaud  <justin_michaud@apple.com>
>+
>+        CSS Painting API code cleanup
>+        https://bugs.webkit.org/show_bug.cgi?id=192480
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        No new tests, since the existing tests should cover it.
>+
>+        * bindings/js/JSDOMWrapper.cpp:
>+        (WebCore::outputConstraintSubspaceFor):
>+        (WebCore::globalObjectOutputConstraintSubspaceFor):
>+        * bindings/js/JSWorkletGlobalScopeBase.cpp:
>+        (WebCore::toJS):
>+        * css/CSSPaintCallback.h:
>+        * platform/graphics/CustomPaintImage.cpp:
>+        (WebCore::CustomPaintImage::doCustomPaint):
>+        * platform/graphics/CustomPaintImage.h:
>+        * rendering/style/RenderStyle.cpp:
>+        (WebCore::RenderStyle::addCustomPaintWatchProperty):
>+        (WebCore::changedCustomPaintWatchedProperty):
>+        (WebCore::RenderStyle::changeRequiresRepaint const):
>+        * worklets/PaintWorkletGlobalScope.cpp:
>+        (WebCore::PaintWorkletGlobalScope::registerPaint):
>+        * worklets/PaintWorkletGlobalScope.h:
>+
> 2018-12-06  Alex Christensen  <achristensen@webkit.org>
> 
>         Remove unused LoaderStrategy::storeDerivedDataToCache and associated dead code
>diff --git a/Source/WebCore/bindings/js/JSDOMWrapper.cpp b/Source/WebCore/bindings/js/JSDOMWrapper.cpp
>index 9bf0b1c67dd75246b8e84a429e7b271adb44f958..e9525a46d88e1a5b024a5df3f3317339ed31210c 100644
>--- a/Source/WebCore/bindings/js/JSDOMWrapper.cpp
>+++ b/Source/WebCore/bindings/js/JSDOMWrapper.cpp
>@@ -35,7 +35,6 @@
> #include <JavaScriptCore/Error.h>
> 
> namespace WebCore {
>-using namespace JSC;
> 
> STATIC_ASSERT_IS_TRIVIALLY_DESTRUCTIBLE(JSDOMObject);
> 
>@@ -45,12 +44,12 @@ JSDOMObject::JSDOMObject(JSC::Structure* structure, JSC::JSGlobalObject& globalO
>     ASSERT(globalObject.classInfo() == JSDOMGlobalObject::info() || scriptExecutionContext() || globalObject.classInfo() == JSRemoteDOMWindow::info());
> }
> 
>-CompleteSubspace* outputConstraintSubspaceFor(VM& vm)
>+JSC::CompleteSubspace* outputConstraintSubspaceFor(JSC::VM& vm)
> {
>     return &static_cast<JSVMClientData*>(vm.clientData)->outputConstraintSpace();
> }
> 
>-CompleteSubspace* globalObjectOutputConstraintSubspaceFor(VM& vm)
>+JSC::CompleteSubspace* globalObjectOutputConstraintSubspaceFor(JSC::VM& vm)
> {
>     return &static_cast<JSVMClientData*>(vm.clientData)->globalObjectOutputConstraintSpace();
> }
>diff --git a/Source/WebCore/bindings/js/JSWorkletGlobalScopeBase.cpp b/Source/WebCore/bindings/js/JSWorkletGlobalScopeBase.cpp
>index 7cd80e74157ffacfdaed0c64ea99bce442c886e2..71513a7249223a972b0a0b75f931601a5e989a2d 100644
>--- a/Source/WebCore/bindings/js/JSWorkletGlobalScopeBase.cpp
>+++ b/Source/WebCore/bindings/js/JSWorkletGlobalScopeBase.cpp
>@@ -128,9 +128,11 @@ JSValue toJS(ExecState* exec, JSDOMGlobalObject*, WorkletGlobalScope& workletGlo
> 
> JSValue toJS(ExecState*, WorkletGlobalScope& workletGlobalScope)
> {
>-    ASSERT(workletGlobalScope.script());
>+    if (!workletGlobalScope.script())
>+        return jsUndefined();
>     auto* contextWrapper = workletGlobalScope.script()->workletGlobalScopeWrapper();
>-    ASSERT(contextWrapper);
>+    if (!contextWrapper)
>+        return jsUndefined();
>     return contextWrapper->proxy();
> }
> 
>diff --git a/Source/WebCore/css/CSSPaintCallback.h b/Source/WebCore/css/CSSPaintCallback.h
>index 0c03f0afbd712449edd07136d653577aed74df64..b877f0306f803cf98c6b3602032d742f9a29d9ba 100644
>--- a/Source/WebCore/css/CSSPaintCallback.h
>+++ b/Source/WebCore/css/CSSPaintCallback.h
>@@ -31,10 +31,13 @@
> #include "CSSPaintSize.h"
> #include "CallbackResult.h"
> #include "StylePropertyMapReadOnly.h"
>-#include <JavaScriptCore/JSCJSValue.h>
> #include <wtf/RefCounted.h>
> #include <wtf/WeakPtr.h>
> 
>+namespace JSC {
>+class JSValue;
>+} // namespace JSC
>+
> namespace WebCore {
> class PaintRenderingContext2D;
> 
>diff --git a/Source/WebCore/platform/graphics/CustomPaintImage.cpp b/Source/WebCore/platform/graphics/CustomPaintImage.cpp
>index fa90d9f1c98972ac3af0d05fcf11aa228a1158dc..b8f930c81a0a00bc427501a21de714017f66168a 100644
>--- a/Source/WebCore/platform/graphics/CustomPaintImage.cpp
>+++ b/Source/WebCore/platform/graphics/CustomPaintImage.cpp
>@@ -62,17 +62,15 @@ ImageDrawResult CustomPaintImage::doCustomPaint(GraphicsContext& destContext, co
>     if (!m_element || !m_element->element() || !m_paintDefinition)
>         return ImageDrawResult::DidNothing;
> 
>-    JSC::JSValue paintConstructor(m_paintDefinition->paintConstructor);
>+    JSC::JSValue paintConstructor = m_paintDefinition->paintConstructor;
> 
>     if (!paintConstructor)
>         return ImageDrawResult::DidNothing;
> 
>-    auto& paintCallback = m_paintDefinition->paintCallback.get();
>-
>     ASSERT(!m_element->needsLayout());
>     ASSERT(!m_element->element()->document().needsStyleRecalc());
> 
>-    JSCSSPaintCallback& callback = static_cast<JSCSSPaintCallback&>(paintCallback);
>+    JSCSSPaintCallback& callback = static_cast<JSCSSPaintCallback&>(m_paintDefinition->paintCallback.get());
>     auto* scriptExecutionContext = callback.scriptExecutionContext();
>     if (!scriptExecutionContext)
>         return ImageDrawResult::DidNothing;
>@@ -122,14 +120,14 @@ ImageDrawResult CustomPaintImage::doCustomPaint(GraphicsContext& destContext, co
> 
>     auto& state = *globalObject.globalExec();
>     JSC::ArgList noArgs;
>-    JSC::JSValue thisObject = { JSC::construct(&state, WTFMove(paintConstructor), noArgs, "Failed to construct paint class") };
>+    JSC::JSValue thisObject = { JSC::construct(&state, paintConstructor, noArgs, "Failed to construct paint class") };
> 
>     if (UNLIKELY(scope.exception())) {
>         reportException(&state, scope.exception());
>         return ImageDrawResult::DidNothing;
>     }
> 
>-    auto result = paintCallback.handleEvent(WTFMove(thisObject), *context, size, propertyMap, m_arguments);
>+    auto result = callback.handleEvent(WTFMove(thisObject), *context, size, propertyMap, m_arguments);
>     if (result.type() != CallbackResultType::Success)
>         return ImageDrawResult::DidNothing;
> 
>diff --git a/Source/WebCore/platform/graphics/CustomPaintImage.h b/Source/WebCore/platform/graphics/CustomPaintImage.h
>index 68b40c99ecab3a5e3e0c92b2b013d78651a72167..9db60cb7d7f0e7b677990339ebd5365ab608a154 100644
>--- a/Source/WebCore/platform/graphics/CustomPaintImage.h
>+++ b/Source/WebCore/platform/graphics/CustomPaintImage.h
>@@ -29,8 +29,7 @@
> 
> #include "GeneratedImage.h"
> #include "PaintWorkletGlobalScope.h"
>-#include <JavaScriptCore/JSObject.h>
>-#include <JavaScriptCore/Weak.h>
>+
> #include <wtf/WeakPtr.h>
> 
> namespace WebCore {
>diff --git a/Source/WebCore/rendering/style/RenderStyle.cpp b/Source/WebCore/rendering/style/RenderStyle.cpp
>index faf9ca08b554d882d68b2345c8f5a63757b1d12d..df39cb23845e73bf51ac2b061cf79c935f35b94a 100644
>--- a/Source/WebCore/rendering/style/RenderStyle.cpp
>+++ b/Source/WebCore/rendering/style/RenderStyle.cpp
>@@ -972,32 +972,11 @@ void RenderStyle::addCustomPaintWatchProperty(const String& name)
>         data.customPaintWatchedProperties = std::make_unique<HashSet<String>>();
>     data.customPaintWatchedProperties->add(name);
> }
>-#endif
> 
>-bool RenderStyle::changeRequiresRepaint(const RenderStyle& other, OptionSet<StyleDifferenceContextSensitiveProperty>& changedContextSensitiveProperties) const
>+inline static bool changedCustomPaintWatchedProperty(const RenderStyle& a, const StyleRareNonInheritedData& aData, const RenderStyle& b, const StyleRareNonInheritedData& bData)
> {
>-    if (!requiresPainting(*this) && !requiresPainting(other))
>-        return false;
>-
>-    if (m_inheritedFlags.visibility != other.m_inheritedFlags.visibility
>-        || m_inheritedFlags.printColorAdjust != other.m_inheritedFlags.printColorAdjust
>-        || m_inheritedFlags.insideLink != other.m_inheritedFlags.insideLink
>-        || m_inheritedFlags.insideDefaultButton != other.m_inheritedFlags.insideDefaultButton
>-        || m_surroundData->border != other.m_surroundData->border
>-        || !m_backgroundData->isEquivalentForPainting(*other.m_backgroundData))
>-        return true;
>-
>-    if (m_rareNonInheritedData.ptr() != other.m_rareNonInheritedData.ptr()
>-        && rareNonInheritedDataChangeRequiresRepaint(*m_rareNonInheritedData, *other.m_rareNonInheritedData, changedContextSensitiveProperties))
>-        return true;
>-
>-    if (m_rareInheritedData.ptr() != other.m_rareInheritedData.ptr()
>-        && rareInheritedDataChangeRequiresRepaint(*m_rareInheritedData, *other.m_rareInheritedData))
>-        return true;
>-
>-#if ENABLE(CSS_PAINTING_API)
>-    auto* propertiesA = m_rareNonInheritedData.ptr()->customPaintWatchedProperties.get();
>-    auto* propertiesB = other.m_rareNonInheritedData.ptr()->customPaintWatchedProperties.get();
>+    auto* propertiesA = aData.customPaintWatchedProperties.get();
>+    auto* propertiesB = bData.customPaintWatchedProperties.get();
> 
>     if (UNLIKELY(propertiesA || propertiesB)) {
>         // FIXME: We should not need to use ComputedStyleExtractor here.
>@@ -1010,15 +989,15 @@ bool RenderStyle::changeRequiresRepaint(const RenderStyle& other, OptionSet<Styl
>             for (auto& name : *watchPropertiesMap) {
>                 RefPtr<CSSValue> valueA;
>                 RefPtr<CSSValue> valueB;
>-                if (isCustomPropertyName(name) && getCustomProperty(name) && other.getCustomProperty(name)) {
>-                    valueA = CSSCustomPropertyValue::create(*getCustomProperty(name));
>-                    valueB = CSSCustomPropertyValue::create(*other.getCustomProperty(name));
>+                if (isCustomPropertyName(name) && a.getCustomProperty(name) && b.getCustomProperty(name)) {
>+                    valueA = CSSCustomPropertyValue::create(*a.getCustomProperty(name));
>+                    valueB = CSSCustomPropertyValue::create(*b.getCustomProperty(name));
>                 } else {
>                     CSSPropertyID propertyID = cssPropertyID(name);
>                     if (!propertyID)
>                         continue;
>-                    valueA = extractor.valueForPropertyinStyle(*this, propertyID);
>-                    valueB = extractor.valueForPropertyinStyle(other, propertyID);
>+                    valueA = extractor.valueForPropertyinStyle(a, propertyID);
>+                    valueB = extractor.valueForPropertyinStyle(b, propertyID);
>                 }
> 
>                 if ((valueA && !valueB) || (!valueA && valueB))
>@@ -1032,6 +1011,35 @@ bool RenderStyle::changeRequiresRepaint(const RenderStyle& other, OptionSet<Styl
>             }
>         }
>     }
>+
>+    return false;
>+}
>+#endif
>+
>+bool RenderStyle::changeRequiresRepaint(const RenderStyle& other, OptionSet<StyleDifferenceContextSensitiveProperty>& changedContextSensitiveProperties) const
>+{
>+    if (!requiresPainting(*this) && !requiresPainting(other))
>+        return false;
>+
>+    if (m_inheritedFlags.visibility != other.m_inheritedFlags.visibility
>+        || m_inheritedFlags.printColorAdjust != other.m_inheritedFlags.printColorAdjust
>+        || m_inheritedFlags.insideLink != other.m_inheritedFlags.insideLink
>+        || m_inheritedFlags.insideDefaultButton != other.m_inheritedFlags.insideDefaultButton
>+        || m_surroundData->border != other.m_surroundData->border
>+        || !m_backgroundData->isEquivalentForPainting(*other.m_backgroundData))
>+        return true;
>+
>+    if (m_rareNonInheritedData.ptr() != other.m_rareNonInheritedData.ptr()
>+        && rareNonInheritedDataChangeRequiresRepaint(*m_rareNonInheritedData, *other.m_rareNonInheritedData, changedContextSensitiveProperties))
>+        return true;
>+
>+    if (m_rareInheritedData.ptr() != other.m_rareInheritedData.ptr()
>+        && rareInheritedDataChangeRequiresRepaint(*m_rareInheritedData, *other.m_rareInheritedData))
>+        return true;
>+
>+#if ENABLE(CSS_PAINTING_API)
>+    if (changedCustomPaintWatchedProperty(*this, *m_rareNonInheritedData, other, *other.m_rareNonInheritedData))
>+        return true;
> #endif
> 
>     return false;
>diff --git a/Source/WebCore/worklets/PaintWorkletGlobalScope.cpp b/Source/WebCore/worklets/PaintWorkletGlobalScope.cpp
>index 3e3d9561f26673af856295a802c7a0466bd044e8..fb483423c4254ea87ea7addd99a68ed7b1cf91f7 100644
>--- a/Source/WebCore/worklets/PaintWorkletGlobalScope.cpp
>+++ b/Source/WebCore/worklets/PaintWorkletGlobalScope.cpp
>@@ -141,6 +141,7 @@ ExceptionOr<void> PaintWorkletGlobalScope::registerPaint(JSC::ExecState& state,
> 
>     // FIXME: construct documentDefinition (step 22).
> 
>+    // FIXME: we should only repaint affected custom paint images <https://bugs.webkit.org/show_bug.cgi?id=192322>.
>     if (responsibleDocument() && responsibleDocument()->renderView())
>         responsibleDocument()->renderView()->repaintRootContents();
> 
>diff --git a/Source/WebCore/worklets/PaintWorkletGlobalScope.h b/Source/WebCore/worklets/PaintWorkletGlobalScope.h
>index f98e577e3d49a1394061637cdd12252d5bbfaa52..7ab6b5161d55332e790c2942fb1a9bb1574c5d30 100644
>--- a/Source/WebCore/worklets/PaintWorkletGlobalScope.h
>+++ b/Source/WebCore/worklets/PaintWorkletGlobalScope.h
>@@ -46,12 +46,12 @@ public:
>     ExceptionOr<void> registerPaint(JSC::ExecState&, JSDOMGlobalObject&, const String& name, JSC::Strong<JSC::JSObject> paintConstructor);
>     double devicePixelRatio() const;
> 
>+    // All paint definitions must be destroyed before the vm is destroyed, because otherwise they will point to freed memory.
>     struct PaintDefinition : public CanMakeWeakPtr<PaintDefinition> {
>         PaintDefinition(const AtomicString& name, JSC::JSObject* paintConstructor, Ref<CSSPaintCallback>&&, Vector<String>&& inputProperties, Vector<String>&& inputArguments);
> 
>         const AtomicString name;
>-        // This map must be cleared before the vm is destroyed!
>-        JSC::JSObject* paintConstructor { nullptr };
>+        const JSC::JSObject* const paintConstructor;
>         const Ref<CSSPaintCallback> paintCallback;
>         const Vector<String> inputProperties;
>         const Vector<String> inputArguments;

Actions: View | Formatted Diff | Diff

Attachments on bug 192480: 356763