- Revision
- 277631
- Author
- wenson_hs...@apple.com
- Date
- 2021-05-17 19:33:16 -0700 (Mon, 17 May 2021)
Log Message
[GPU Process] Object identifiers with the deleted value should cause MESSAGE_CHECKs
https://bugs.webkit.org/show_bug.cgi?id=225886
rdar://78114708
Reviewed by Chris Dumez.
Source/WebCore:
Implement some stricter validation around object identifiers in when decoding display list items in the GPU
Process. Currently, we only check for the empty value (i.e. raw identifier value of 0) when iterating over these
items, but treat an identifier with the deleted value as valid; instead, we should be treating items with either
empty or deleted identifiers as invalid.
To address this, we introduce a new helper method, `ObjectIdentifier::isValid`, and turn existing checks for
`!!identifier` into `identifier.isValid()`.
Test: DisplayListTests.InlineItemValidationFailure
* platform/graphics/displaylists/DisplayListItems.h:
(WebCore::DisplayList::ClipToImageBuffer::isValid const):
(WebCore::DisplayList::DrawImageBuffer::isValid const):
(WebCore::DisplayList::DrawNativeImage::isValid const):
(WebCore::DisplayList::DrawPattern::isValid const):
(WebCore::DisplayList::PaintFrameForMedia::isValid const):
(WebCore::DisplayList::FlushContext::isValid const):
(WebCore::DisplayList::MetaCommandChangeItemBuffer::isValid const):
(WebCore::DisplayList::MetaCommandChangeDestinationImageBuffer::isValid const):
Source/WebKit:
See WebCore/ChangeLog for more details. Use `ObjectIdentifier::isValid()` instead of just checking for the empty
value, when determining whether an object identifier should trigger a message check to the web process.
* GPUProcess/graphics/RemoteRenderingBackend.cpp:
(WebKit::RemoteRenderingBackend::nextDestinationImageBufferAfterApplyingDisplayLists):
Source/WTF:
See WebCore/ChangeLog for more details. Add a helper method on `ObjectIdentifier` that returns true only if it
is the empty value or deleted value.
* wtf/ObjectIdentifier.h:
(WTF::ObjectIdentifier::isValid const):
Tools:
Adjust an existing API test to verify that the deleted object identifier value triggers an inline item decoding
failure.
* TestWebKitAPI/Tests/WebCore/cg/DisplayListTestsCG.cpp:
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (277630 => 277631)
--- trunk/Source/WTF/ChangeLog 2021-05-18 02:19:47 UTC (rev 277630)
+++ trunk/Source/WTF/ChangeLog 2021-05-18 02:33:16 UTC (rev 277631)
@@ -1,3 +1,17 @@
+2021-05-17 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [GPU Process] Object identifiers with the deleted value should cause MESSAGE_CHECKs
+ https://bugs.webkit.org/show_bug.cgi?id=225886
+ rdar://78114708
+
+ Reviewed by Chris Dumez.
+
+ See WebCore/ChangeLog for more details. Add a helper method on `ObjectIdentifier` that returns true only if it
+ is the empty value or deleted value.
+
+ * wtf/ObjectIdentifier.h:
+ (WTF::ObjectIdentifier::isValid const):
+
2021-05-17 Alex Christensen <achristen...@webkit.org>
Use kAudioObjectPropertyElementMain where available
Modified: trunk/Source/WTF/wtf/ObjectIdentifier.h (277630 => 277631)
--- trunk/Source/WTF/wtf/ObjectIdentifier.h 2021-05-18 02:19:47 UTC (rev 277630)
+++ trunk/Source/WTF/wtf/ObjectIdentifier.h 2021-05-18 02:33:16 UTC (rev 277631)
@@ -63,6 +63,7 @@
ObjectIdentifier(HashTableDeletedValueType) : m_identifier(hashTableDeletedValue()) { }
bool isHashTableDeletedValue() const { return m_identifier == hashTableDeletedValue(); }
+ bool isValid() const { return isValidIdentifier(m_identifier); }
template<typename Encoder> void encode(Encoder& encoder) const
{
Modified: trunk/Source/WebCore/ChangeLog (277630 => 277631)
--- trunk/Source/WebCore/ChangeLog 2021-05-18 02:19:47 UTC (rev 277630)
+++ trunk/Source/WebCore/ChangeLog 2021-05-18 02:33:16 UTC (rev 277631)
@@ -1,3 +1,31 @@
+2021-05-17 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [GPU Process] Object identifiers with the deleted value should cause MESSAGE_CHECKs
+ https://bugs.webkit.org/show_bug.cgi?id=225886
+ rdar://78114708
+
+ Reviewed by Chris Dumez.
+
+ Implement some stricter validation around object identifiers in when decoding display list items in the GPU
+ Process. Currently, we only check for the empty value (i.e. raw identifier value of 0) when iterating over these
+ items, but treat an identifier with the deleted value as valid; instead, we should be treating items with either
+ empty or deleted identifiers as invalid.
+
+ To address this, we introduce a new helper method, `ObjectIdentifier::isValid`, and turn existing checks for
+ `!!identifier` into `identifier.isValid()`.
+
+ Test: DisplayListTests.InlineItemValidationFailure
+
+ * platform/graphics/displaylists/DisplayListItems.h:
+ (WebCore::DisplayList::ClipToImageBuffer::isValid const):
+ (WebCore::DisplayList::DrawImageBuffer::isValid const):
+ (WebCore::DisplayList::DrawNativeImage::isValid const):
+ (WebCore::DisplayList::DrawPattern::isValid const):
+ (WebCore::DisplayList::PaintFrameForMedia::isValid const):
+ (WebCore::DisplayList::FlushContext::isValid const):
+ (WebCore::DisplayList::MetaCommandChangeItemBuffer::isValid const):
+ (WebCore::DisplayList::MetaCommandChangeDestinationImageBuffer::isValid const):
+
2021-05-17 Fujii Hironori <hironori.fu...@sony.com>
[Win] Unreviewed debug build fix for r277601.
Modified: trunk/Source/WebCore/platform/graphics/displaylists/DisplayListItems.h (277630 => 277631)
--- trunk/Source/WebCore/platform/graphics/displaylists/DisplayListItems.h 2021-05-18 02:19:47 UTC (rev 277630)
+++ trunk/Source/WebCore/platform/graphics/displaylists/DisplayListItems.h 2021-05-18 02:33:16 UTC (rev 277631)
@@ -768,7 +768,7 @@
RenderingResourceIdentifier imageBufferIdentifier() const { return m_imageBufferIdentifier; }
FloatRect destinationRect() const { return m_destinationRect; }
- bool isValid() const { return !!m_imageBufferIdentifier; }
+ bool isValid() const { return m_imageBufferIdentifier.isValid(); }
void apply(GraphicsContext&, WebCore::ImageBuffer&) const;
@@ -1016,7 +1016,7 @@
FloatRect destinationRect() const { return m_destinationRect; }
ImagePaintingOptions options() const { return m_options; }
// FIXME: We might want to validate ImagePaintingOptions.
- bool isValid() const { return !!m_imageBufferIdentifier; }
+ bool isValid() const { return m_imageBufferIdentifier.isValid(); }
void apply(GraphicsContext&, WebCore::ImageBuffer&) const;
@@ -1051,7 +1051,7 @@
const FloatRect& source() const { return m_srcRect; }
const FloatRect& destinationRect() const { return m_destinationRect; }
// FIXME: We might want to validate ImagePaintingOptions.
- bool isValid() const { return !!m_imageIdentifier; }
+ bool isValid() const { return m_imageIdentifier.isValid(); }
NO_RETURN_DUE_TO_ASSERT void apply(GraphicsContext&) const;
void apply(GraphicsContext&, NativeImage&) const;
@@ -1083,7 +1083,7 @@
FloatPoint phase() const { return m_phase; }
FloatSize spacing() const { return m_spacing; }
// FIXME: We might want to validate ImagePaintingOptions.
- bool isValid() const { return !!m_imageIdentifier; }
+ bool isValid() const { return m_imageIdentifier.isValid(); }
NO_RETURN_DUE_TO_ASSERT void apply(GraphicsContext&) const;
void apply(GraphicsContext&, NativeImage&) const;
@@ -2018,7 +2018,7 @@
const FloatRect& destination() const { return m_destination; }
MediaPlayerIdentifier identifier() const { return m_identifier; }
- bool isValid() const { return !!m_identifier; }
+ bool isValid() const { return m_identifier.isValid(); }
NO_RETURN_DUE_TO_ASSERT void apply(GraphicsContext&) const;
@@ -2251,7 +2251,7 @@
}
FlushIdentifier identifier() const { return m_identifier; }
- bool isValid() const { return !!m_identifier; }
+ bool isValid() const { return m_identifier.isValid(); }
void apply(GraphicsContext&) const;
@@ -2273,7 +2273,7 @@
}
ItemBufferIdentifier identifier() const { return m_identifier; }
- bool isValid() const { return !!m_identifier; }
+ bool isValid() const { return m_identifier.isValid(); }
private:
ItemBufferIdentifier m_identifier;
@@ -2291,7 +2291,7 @@
}
RenderingResourceIdentifier identifier() const { return m_identifier; }
- bool isValid() const { return !!m_identifier; }
+ bool isValid() const { return m_identifier.isValid(); }
private:
RenderingResourceIdentifier m_identifier;
Modified: trunk/Source/WebKit/ChangeLog (277630 => 277631)
--- trunk/Source/WebKit/ChangeLog 2021-05-18 02:19:47 UTC (rev 277630)
+++ trunk/Source/WebKit/ChangeLog 2021-05-18 02:33:16 UTC (rev 277631)
@@ -1,3 +1,17 @@
+2021-05-17 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [GPU Process] Object identifiers with the deleted value should cause MESSAGE_CHECKs
+ https://bugs.webkit.org/show_bug.cgi?id=225886
+ rdar://78114708
+
+ Reviewed by Chris Dumez.
+
+ See WebCore/ChangeLog for more details. Use `ObjectIdentifier::isValid()` instead of just checking for the empty
+ value, when determining whether an object identifier should trigger a message check to the web process.
+
+ * GPUProcess/graphics/RemoteRenderingBackend.cpp:
+ (WebKit::RemoteRenderingBackend::nextDestinationImageBufferAfterApplyingDisplayLists):
+
2021-05-17 Kate Cheney <katherine_che...@apple.com>
WebFrameLoaderClient::dispatchWillSendRequest sometimes resets app-bound value
Modified: trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp (277630 => 277631)
--- trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp 2021-05-18 02:19:47 UTC (rev 277630)
+++ trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp 2021-05-18 02:33:16 UTC (rev 277631)
@@ -258,7 +258,7 @@
MESSAGE_CHECK_WITH_RETURN_VALUE(sizeToRead, nullptr, "No unread bytes when resuming display list processing");
auto newDestinationIdentifier = makeObjectIdentifier<RenderingResourceIdentifierType>(resumeReadingInfo->destination);
- MESSAGE_CHECK_WITH_RETURN_VALUE(newDestinationIdentifier, nullptr, "Invalid image buffer destination when resuming display list processing");
+ MESSAGE_CHECK_WITH_RETURN_VALUE(newDestinationIdentifier.isValid(), nullptr, "Invalid image buffer destination when resuming display list processing");
destination = makeRefPtr(m_remoteResourceCache.cachedImageBuffer(newDestinationIdentifier));
MESSAGE_CHECK_WITH_RETURN_VALUE(destination, nullptr, "Missing image buffer destination when resuming display list processing");
Modified: trunk/Tools/ChangeLog (277630 => 277631)
--- trunk/Tools/ChangeLog 2021-05-18 02:19:47 UTC (rev 277630)
+++ trunk/Tools/ChangeLog 2021-05-18 02:33:16 UTC (rev 277631)
@@ -1,3 +1,17 @@
+2021-05-17 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [GPU Process] Object identifiers with the deleted value should cause MESSAGE_CHECKs
+ https://bugs.webkit.org/show_bug.cgi?id=225886
+ rdar://78114708
+
+ Reviewed by Chris Dumez.
+
+ Adjust an existing API test to verify that the deleted object identifier value triggers an inline item decoding
+ failure.
+
+ * TestWebKitAPI/Tests/WebCore/cg/DisplayListTestsCG.cpp:
+ (TestWebKitAPI::TEST):
+
2021-05-17 Kate Cheney <katherine_che...@apple.com>
WebFrameLoaderClient::dispatchWillSendRequest sometimes resets app-bound value
Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/cg/DisplayListTestsCG.cpp (277630 => 277631)
--- trunk/Tools/TestWebKitAPI/Tests/WebCore/cg/DisplayListTestsCG.cpp 2021-05-18 02:19:47 UTC (rev 277630)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/cg/DisplayListTestsCG.cpp 2021-05-18 02:33:16 UTC (rev 277631)
@@ -145,17 +145,24 @@
auto cgContext = adoptCF(CGBitmapContextCreate(nullptr, contextWidth, contextHeight, 8, 4 * contextWidth, colorSpace.get(), kCGImageAlphaPremultipliedLast));
GraphicsContext context { cgContext.get() };
- DisplayList list;
- ReadingClient reader;
- list.setItemBufferReadingClient(&reader);
- list.append<FlushContext>(FlushIdentifier { });
+ auto runTestWithInvalidIdentifier = [&](FlushIdentifier identifier) {
+ EXPECT_FALSE(identifier.isValid());
- Replayer replayer { context, list };
- auto result = replayer.replay();
- EXPECT_EQ(result.numberOfBytesRead, 0U);
- EXPECT_EQ(result.nextDestinationImageBuffer, WTF::nullopt);
- EXPECT_EQ(result.missingCachedResourceIdentifier, WTF::nullopt);
- EXPECT_EQ(result.reasonForStopping, StopReplayReason::InvalidItemOrExtent);
+ DisplayList list;
+ ReadingClient reader;
+ list.setItemBufferReadingClient(&reader);
+ list.append<FlushContext>(identifier);
+
+ Replayer replayer { context, list };
+ auto result = replayer.replay();
+ EXPECT_EQ(result.numberOfBytesRead, 0U);
+ EXPECT_EQ(result.nextDestinationImageBuffer, WTF::nullopt);
+ EXPECT_EQ(result.missingCachedResourceIdentifier, WTF::nullopt);
+ EXPECT_EQ(result.reasonForStopping, StopReplayReason::InvalidItemOrExtent);
+ };
+
+ runTestWithInvalidIdentifier(FlushIdentifier { });
+ runTestWithInvalidIdentifier(FlushIdentifier { WTF::HashTableDeletedValue });
}
} // namespace TestWebKitAPI
