WebKit Bugzilla
Attachment 356763 Details for
Bug 192480
: CSS Painting API code cleanup
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[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:
Patch
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;
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 192480
: 356763