Move TextIterator::rangeFromLocationAndLength off of live ranges
Created attachment 394236 [details] Patch for EWS (includes all of the patch from bug 209207 too)
Created attachment 394316 [details] Patch
Created attachment 394757 [details] Patch
Created attachment 394767 [details] Patch
Created attachment 394782 [details] Patch
Created attachment 394821 [details] Patch
Created attachment 394846 [details] Patch
Created attachment 394848 [details] Patch
Created attachment 394851 [details] Patch
Created attachment 394853 [details] Patch
Created attachment 394859 [details] Patch
Created attachment 394860 [details] Patch
OK, should be ready for review now.
This patch also resolves bug 205314; should probably mention that in the change log.
Windows bot failure isn’t real. Just conflict with a part of my latch with Dave Kilzer’s identical fix for a compile error on newer macOS that prevented the bot from applying the patch. I could upload a new patch to address it but I don’t think I should.
Created attachment 394869 [details] Patch
Comment on attachment 394869 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394869&action=review > Source/WebCore/editing/CharacterRange.h:40 > +using CharacterCount = std::size_t; Why is this alias helpful? Why not just use size_t? (I know it is not new in this patch) > Source/WebCore/editing/CharacterRange.h:44 > + CharacterCount location { 0 }; > + CharacterCount length { 0 }; It is bit weird that a location type is called "count". > Source/WebCore/editing/CharacterRange.h:79 > +inline Optional<CharacterRange> makeCharacterRange(CFRange range) > +{ > + ASSERT(range.location == kCFNotFound || range.location >= 0); > + ASSERT(range.length >= 0); > + if (range.location < 0 || range.length < 0) > + return WTF::nullopt; > + return CharacterRange { static_cast<CharacterCount>(range.location), static_cast<CharacterCount>(range.length) }; > +} Most clients are just unwrapping the Optional without any checking. It might be better to add a separate function for the few cases where kCFNotFound is actually a valid input (makeCharacterRangeIfValid or something) and make the regular version assert and return a plain CharacterRange. > Source/WebCore/editing/TextCheckingHelper.cpp:67 > + badGrammar.range.location = checkLocation + badGrammarLocation; > + badGrammar.range.length = badGrammarLength; badGrammar.range = { checkLocation + badGrammarLocation, badGrammarLength }; ? > Source/WebCore/editing/TextCheckingHelper.cpp:101 > + misspelling.range.location = wordStart + misspellingLocation; > + misspelling.range.length = misspellingLength; Similarly > Source/WebCore/editing/TextCheckingHelper.cpp:279 > + CharacterRange mispellingCharacterRange; > + mispellingCharacterRange.location = currentChunkOffset + misspellingLocation; > + mispellingCharacterRange.length = misspellingLength; auto mispellingCharacterRange = CharacterRange { currentChunkOffset + misspellingLocation, misspellingLength }; ? > Source/WebCore/editing/TextCheckingHelper.cpp:317 > + outGrammarDetail.range.location = 0; > + outGrammarDetail.range.length = 0; outGrammarDetail.range = { }; > Source/WebCore/editing/TextCheckingHelper.cpp:471 > + outGrammarDetail.range.location = 0; > + outGrammarDetail.range.length = 0; outGrammarDetail.range = { }; > Source/WebKit/UIProcess/WebGrammarDetail.cpp:48 > + m_grammarDetail.range.location = location; > + m_grammarDetail.range.length = length; m_grammarDetail.range = { location, length }; > Source/WebKit/UIProcess/gtk/TextCheckerGtk.cpp:273 > + misspellingResult.range.location = offset + misspellingLocation; > + misspellingResult.range.length = misspellingLength; etc > Source/WebKit/WebProcess/WebPage/WebPage.cpp:5435 > + CharacterRange characterRange; > + characterRange.location = WebCore::characterCount(*surroundingRange) + offset; > + characterRange.length = characterCount; etc > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:4081 > + CharacterRange newSelectionRange; > + newSelectionRange.location = newSelectionLocation.unsafeGet(); > + newSelectionRange.length = newSelectionLength.unsafeGet(); etc > Source/WebKitLegacy/win/WebCoreSupport/WebEditorClient.cpp:805 > + detail.range.location = location; > + detail.range.length = length; auto characterRange = CharacterRange { location, length } > Source/WebKitLegacy/win/WebView.cpp:7771 > + CharacterRange characterRange; > + characterRange.location = location; > + characterRange.length = length; How about just auto characterRange = CharacterRange { location, length }
I now think this breaks the Mac Catalyst build and I won’t land it before learning how to at least build that configuration locally. Also I think we need this configuration in EWS.
Comment on attachment 394869 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394869&action=review Thank you so much for the review! I really appreciate it. I could use a little help deciding what to do. A couple of your comments I can definitely do, the others open questions and I would like style advice. >> Source/WebCore/editing/CharacterRange.h:40 >> +using CharacterCount = std::size_t; > > Why is this alias helpful? Why not just use size_t? > > (I know it is not new in this patch) I think it’s helpful to make the semantics clearer. It’s like a variable name. Otherwise it just seems random what is int and what is size_t. But not 100% sure. I could definitely retire this type with a global replace. Separate thought: maybe this should be std::uint64_t instead of std::size_t? >> Source/WebCore/editing/CharacterRange.h:44 >> + CharacterCount length { 0 }; > > It is bit weird that a location type is called "count". It’s the count of the number of characters before that location. I am open to other ideas about the names. Sometimes people call this an index or an offset. >> Source/WebCore/editing/CharacterRange.h:79 >> +} > > Most clients are just unwrapping the Optional without any checking. It might be better to add a separate function for the few cases where kCFNotFound is actually a valid input (makeCharacterRangeIfValid or something) and make the regular version assert and return a plain CharacterRange. Thanks for the suggestion! Seems OK, that’s how I started, and I’d be willing to go back to that approach. If I was willing to just assert I could make this a constructor of CharacterRange instead of a named function. Not sure how you feel about that? Makes the code more elegant, but also elegantly hides the fact that we might need to check for not found. I don’t really need to provide a helper function; can just check for "not found" in all the places that need it. But I was thinking this Optional might help people remember the check in future code they write? >> Source/WebCore/editing/TextCheckingHelper.cpp:67 >> + badGrammar.range.length = badGrammarLength; > > badGrammar.range = { checkLocation + badGrammarLocation, badGrammarLength }; > > ? Can’t do exactly that because checkLocation is unsigned, badGrammarLocation is int, badGrammarLength is int, and ranges want CharacterCount/std::size_t. We could write: badGrammar.range = CharacterRange(checkLocation + badGrammarLocation, badGrammarLength); If you like that better I’d be willing to go with it. >> Source/WebCore/editing/TextCheckingHelper.cpp:101 >> + misspelling.range.length = misspellingLength; > > Similarly Same issue/choice. >> Source/WebCore/editing/TextCheckingHelper.cpp:279 >> + mispellingCharacterRange.length = misspellingLength; > > auto mispellingCharacterRange = CharacterRange { currentChunkOffset + misspellingLocation, misspellingLength }; > > ? Same thing again, the mix of types. >> Source/WebCore/editing/TextCheckingHelper.cpp:317 >> + outGrammarDetail.range.length = 0; > > outGrammarDetail.range = { }; Will do. But really, out parameters stink and that’s the real fix. If we created a new GrammarDetail everything would be initialized. >> Source/WebCore/editing/TextCheckingHelper.cpp:471 >> + outGrammarDetail.range.length = 0; > > outGrammarDetail.range = { }; OK. >> Source/WebKit/UIProcess/WebGrammarDetail.cpp:48 >> + m_grammarDetail.range.length = length; > > m_grammarDetail.range = { location, length }; Same problem with types here. Could use the CharacterRange(location, length) syntax. >> Source/WebKit/UIProcess/gtk/TextCheckerGtk.cpp:273 >> + misspellingResult.range.length = misspellingLength; > > etc Yup, same thing again. >> Source/WebKit/WebProcess/WebPage/WebPage.cpp:5435 >> + characterRange.length = characterCount; > > etc Same problem with types here. std::size_t + std::int64_t might not yield a std::size_t. >> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:4081 >> + newSelectionRange.length = newSelectionLength.unsafeGet(); > > etc Here it’s int64_t, not std::size_t. >> Source/WebKitLegacy/win/WebCoreSupport/WebEditorClient.cpp:805 >> + detail.range.length = length; > > auto characterRange = CharacterRange { location, length } Here it’s int. >> Source/WebKitLegacy/win/WebView.cpp:7771 >> + characterRange.length = length; > > How about just > > auto characterRange = CharacterRange { location, length } Here it’s unsigned.
Comment on attachment 394869 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394869&action=review >>> Source/WebCore/editing/TextCheckingHelper.cpp:67 >>> + badGrammar.range.length = badGrammarLength; >> >> badGrammar.range = { checkLocation + badGrammarLocation, badGrammarLength }; >> >> ? > > Can’t do exactly that because checkLocation is unsigned, badGrammarLocation is int, badGrammarLength is int, and ranges want CharacterCount/std::size_t. We could write: > > badGrammar.range = CharacterRange(checkLocation + badGrammarLocation, badGrammarLength); > > If you like that better I’d be willing to go with it. Or: badGrammar.range = { static_cast<CharacterCount>(checkLocation + badGrammarLocation), static_cast<CharacterCount>(badGrammarLength) };
> If I was willing to just assert I could make this a constructor of > CharacterRange instead of a named function. Not sure how you feel about > that? Makes the code more elegant, but also elegantly hides the fact that we > might need to check for not found. I think for simple structs it is cleaner to use standalone construction functions that complex constructors
Comment on attachment 394869 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394869&action=review > Source/WebCore/editing/Editor.cpp:2776 > +static void correctSpellcheckingPreservingTextCheckingParagraph(TextCheckingParagraph& paragraph, Range& rangeToReplace, const String& replacement, CharacterCount resultLocation, CharacterCount resultLength) This could take CharacterRange > Source/WebCore/editing/Editor.cpp:3781 > + CharacterCount subrangeOffset = scannerPosition + relativeStartPosition; > + CharacterCount subrangeLength = relativeEndPosition - relativeStartPosition + 1; Could be CharacterRange
Created attachment 394878 [details] Patch
Comment on attachment 394878 [details] Patch Already reviewed. Just need EWS to check various platforms before I land.
(In reply to Antti Koivisto from comment #21) > > If I was willing to just assert I could make this a constructor of > > CharacterRange instead of a named function. Not sure how you feel about > > that? Makes the code more elegant, but also elegantly hides the fact that we > > might need to check for not found. > > I think for simple structs it is cleaner to use standalone construction > functions that complex constructors Oh, I didn't see this comment. I ended up making CharacterRange allow construction with CFRange and NSRange. It seems to have turned out well enough that I won’t change it to makeCharacterRange right now. Could come back and do that later if we have regrets.
Comment on attachment 394869 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394869&action=review >> Source/WebCore/editing/Editor.cpp:3781 >> + CharacterCount subrangeLength = relativeEndPosition - relativeStartPosition + 1; > > Could be CharacterRange Did all the other changes in the patch I just posed. Did this change, too, locally and will include it in the patch I land.
Created attachment 394879 [details] Patch
Committed r259184: <https://trac.webkit.org/changeset/259184>
<rdar://problem/61036152>
Comment on attachment 394879 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394879&action=review > LayoutTests/platform/mac-wk1/TestExpectations:241 > +# FIXME: Should be fixed in Catalina; need to re-enable. Asked someone to look into this. > LayoutTests/platform/mac-wk1/TestExpectations:723 > +fast/images/animated-gif-paint-after-animation.html [ Skip ] > +fast/images/async-image-background-image-repeated.html [ Skip ] > +fast/images/async-image-background-image.html [ Skip ] > +fast/images/async-image-multiple-clients-repaint.html [ Skip ] > +fast/images/composited-animated-gif-outside-viewport.html [ Skip ] > +fast/images/decode-render-animated-image.html [ Skip ] > +fast/images/decode-render-static-image.html [ Skip ] > +fast/images/decoding-attribute-async-small-image.html [ Skip ] > +fast/images/decoding-attribute-dynamic-async-small-image.html [ Skip ] > +fast/images/sprite-sheet-image-draw.html [ Skip ] We've got a lot of unfortunate test expectations... > LayoutTests/platform/mac-wk2/TestExpectations:514 > # RTL Scrollbars are enabled on Sierra WebKit2. This comment is obsolete. > LayoutTests/platform/mac-wk2/TestExpectations:544 > # <rdar://problem/25063128> This is closed as unreproducible. But still happening on some (internal) bots. > LayoutTests/platform/mac-wk2/TestExpectations:-585 > -webkit.org/b/162524 [ Sierra Release ] http/tests/cache/disk-cache/disk-cache-redirect.html [ Pass Timeout ] We need to update the bug. > LayoutTests/platform/mac-wk2/TestExpectations:-602 > -webkit.org/b/163453 [ Sierra Release ] http/tests/security/contentSecurityPolicy/upgrade-insecure-requests/basic-upgrade-cors.https.html [ Pass Timeout ] We need to update the bug. > LayoutTests/platform/mac-wk2/TestExpectations:-614 > -webkit.org/b/167206 [ Sierra Release ] mathml/opentype/large-operators-displaystyle-dynamic.html [ Pass ImageOnlyFailure ] We need to update the bug. > LayoutTests/platform/mac-wk2/TestExpectations:-643 > -webkit.org/b/171939 [ Sierra Release ] media/track/track-cue-rendering-on-resize.html [ Pass Timeout ] > - > -webkit.org/b/171947 [ Sierra Release ] transitions/extra-transition.html [ Pass Failure ] We need to update the bugs. > LayoutTests/platform/mac-wk2/TestExpectations:-649 > -webkit.org/b/172054 [ Sierra Debug ] loader/stateobjects/replacestate-size-iframe.html [ Pass Timeout ] We need to update the bug. > LayoutTests/platform/mac-wk2/TestExpectations:642 > +http/tests/storageAccess/request-and-grant-access-then-navigate-cross-site-should-not-have-access-database.html [ Pass Timeout ] > +http/tests/storageAccess/deny-with-prompt-does-not-preserve-gesture.html [ Skip ] > +http/tests/storageAccess/deny-with-prompt-does-not-preserve-gesture-database.html [ Skip ] Something wrong about these, they are skipped everywhere with a confusing comment saying "only tested on Mac". > LayoutTests/platform/mac-wk2/TestExpectations:-696 > -webkit.org/b/172829 [ Sierra Debug ] loader/stateobjects/replacestate-size.html [ Pass Timeout ] We need to update the bug. > LayoutTests/platform/mac-wk2/TestExpectations:700 > +http/tests/ssl/applepay/ApplePayError.html [ Pass ] > +http/tests/ssl/applepay/ApplePaySessionV3.html [ Pass ] > +http/tests/ssl/applepay/ApplePaySessionV4.html [ Pass ] > +http/tests/ssl/applepay/ApplePaySessionV5.html [ Pass ] > +http/tests/ssl/applepay/ApplePayRequestShippingContactV3.https.html [ Pass ] > +http/tests/ssl/applepay/ApplePayShippingAddressChangeEventErrorsV3.https.html [ Pass ] > +accessibility/mac/apple-pay-labels.html [ Pass ] > +accessibility/mac/apple-pay-session-v4.html [ Pass ] None of this seems to be necessary, there are no other expectations that would skip these tests. > LayoutTests/platform/mac-wk2/TestExpectations:712 > +http/tests/resourceLoadStatistics/cookies-with-and-without-user-interaction.html [ Pass ] > +http/tests/resourceLoadStatistics/cookie-deletion.html [ Pass ] > +http/tests/resourceLoadStatistics/non-prevalent-resources-can-access-cookies-in-a-third-party-context.html [ Pass ] > +http/tests/resourceLoadStatistics/add-blocking-to-redirect.html [ Pass ] > +http/tests/resourceLoadStatistics/do-not-remove-blocking-in-redirect.html [ Pass ] > +http/tests/resourceLoadStatistics/clear-in-memory-and-persistent-store.html [ Pass ] > +http/tests/resourceLoadStatistics/clear-in-memory-and-persistent-store-one-hour.html [ Pass ] > +http/tests/resourceLoadStatistics/grandfathering.html [ Pass ] > +http/tests/resourceLoadStatistics/strip-referrer-to-origin-for-third-party-redirects.html [ Pass ] > +http/tests/resourceLoadStatistics/strip-referrer-to-origin-for-third-party-requests.html [ Pass ] > +http/tests/resourceLoadStatistics/cap-cache-max-age-for-prevalent-resource.html [ Pass ] Not sure why we are doing this one by one, not enabling the whole http/tests/resourceLoadStatistics directory. > LayoutTests/platform/mac-wk2/TestExpectations:-759 > -webkit.org/b/178553 [ HighSierra Release ] imported/w3c/web-platform-tests/html/dom/dynamic-markup-insertion/opening-the-input-stream/011.html [ Skip ] > - > -webkit.org/b/165620 [ Sierra ] http/tests/cache/disk-cache/disk-cache-request-headers.html [ Pass Failure ] We need to update the bugs. > LayoutTests/platform/mac-wk2/TestExpectations:-797 > -webkit.org/b/183860 [ Sierra Release ] http/wpt/service-workers/third-party-registration.html [ Pass Failure ] We need to update the bug. > LayoutTests/platform/mac-wk2/TestExpectations:-857 > -webkit.org/b/191658 [ Sierra Release ] fast/layers/no-clipping-overflow-hidden-added-after-transform.html [ Pass ImageOnlyFailure ] We need to update the bug. > LayoutTests/platform/mac/TestExpectations:1169 > # rdar://problem/27475162 We need to update the bug. > LayoutTests/platform/mac/TestExpectations:1188 > +webkit.org/b/159755 fast/text/emoji-num-glyphs.html [ Pass ] > +webkit.org/b/159755 fast/text/emoji-gender-2-3.html [ Pass ] > +webkit.org/b/159755 fast/text/emoji-gender-2-4.html [ Pass ] > +webkit.org/b/159755 fast/text/emoji-gender-2-5.html [ Pass ] > +webkit.org/b/159755 fast/text/emoji-gender-2-6.html [ Pass ] > +webkit.org/b/159755 fast/text/emoji-gender-2-7.html [ Pass ] > +webkit.org/b/159755 fast/text/emoji-gender-2-8.html [ Pass ] > +webkit.org/b/159755 fast/text/emoji-gender-2-9.html [ Pass ] > + > +webkit.org/b/159755 fast/text/emoji-gender-fe0f-3.html [ Pass ] > +webkit.org/b/159755 fast/text/emoji-gender-fe0f-4.html [ Pass ] > +webkit.org/b/159755 fast/text/emoji-gender-fe0f-5.html [ Pass ] > +webkit.org/b/159755 fast/text/emoji-gender-fe0f-6.html [ Pass ] > +webkit.org/b/159755 fast/text/emoji-gender-fe0f-7.html [ Pass ] > +webkit.org/b/159755 fast/text/emoji-gender-fe0f-8.html [ Pass ] > +webkit.org/b/159755 fast/text/emoji-gender-fe0f-9.html [ Pass ] > +webkit.org/b/159755 fast/text/emoji-gender.html [ Pass ] Hmm, why are we not running these on iOS. > LayoutTests/platform/mac/TestExpectations:-1304 > -webkit.org/b/171272 [ Sierra ] fast/text/kaithi.html [ ImageOnlyFailure ] We need to update the bug. > LayoutTests/platform/mac/TestExpectations:-1341 > -webkit.org/b/173328 [ Sierra ] fast/text/system-font-fallback-emoji.html [ Failure ] > -webkit.org/b/173328 [ Sierra ] fast/text/system-font-fallback.html [ ImageOnlyFailure ] We need to update the bugs. > LayoutTests/platform/mac/TestExpectations:1389 > # <rdar://problem/29031509> REGRESSION? (FontParser-195): svg/W3C-SVG-1.1/fonts-elem-* and svg/W3C-SVG-1.1/text-intro-* tests failing > -[ HighSierra+ ] svg/W3C-SVG-1.1/fonts-elem-01-t.svg [ Failure ] > -[ HighSierra+ ] svg/W3C-SVG-1.1/fonts-elem-02-t.svg [ Failure ] > -[ HighSierra+ ] svg/W3C-SVG-1.1/fonts-elem-03-b.svg [ Failure ] > -[ HighSierra+ ] svg/W3C-SVG-1.1/fonts-elem-07-b.svg [ Failure ] > -[ HighSierra+ ] svg/W3C-SVG-1.1/text-intro-01-t.svg [ Failure ] > -[ HighSierra+ ] svg/W3C-SVG-1.1/text-intro-02-b.svg [ Failure ] > -[ HighSierra+ ] svg/W3C-SVG-1.1/text-intro-03-b.svg [ Failure ] > -[ HighSierra+ ] svg/W3C-SVG-1.1/text-intro-04-t.svg [ Failure ] > -[ HighSierra+ ] svg/batik/text/textEffect3.svg [ Failure ] > -[ HighSierra+ ] svg/batik/text/textPosition2.svg [ Failure ] > -[ HighSierra+ ] svg/custom/acid3-test-77.html [ Failure ] > -[ HighSierra+ ] svg/custom/svg-fonts-fallback.xhtml [ Failure ] > -[ HighSierra+ ] svg/wicd/test-rightsizing-b.xhtml [ Failure ] > -[ HighSierra+ ] fast/css-generated-content/initial-letter-first-line-wrapping.html [ ImageOnlyFailure ] > +svg/W3C-SVG-1.1/fonts-elem-01-t.svg [ Failure ] > +svg/W3C-SVG-1.1/fonts-elem-02-t.svg [ Failure ] > +svg/W3C-SVG-1.1/fonts-elem-03-b.svg [ Failure ] > +svg/W3C-SVG-1.1/fonts-elem-07-b.svg [ Failure ] > +svg/W3C-SVG-1.1/text-intro-01-t.svg [ Failure ] > +svg/W3C-SVG-1.1/text-intro-02-b.svg [ Failure ] > +svg/W3C-SVG-1.1/text-intro-03-b.svg [ Failure ] > +svg/W3C-SVG-1.1/text-intro-04-t.svg [ Failure ] > +svg/batik/text/textEffect3.svg [ Failure ] > +svg/batik/text/textPosition2.svg [ Failure ] > +svg/custom/acid3-test-77.html [ Failure ] > +svg/custom/svg-fonts-fallback.xhtml [ Failure ] > +svg/wicd/test-rightsizing-b.xhtml [ Failure ] > +fast/css-generated-content/initial-letter-first-line-wrapping.html [ ImageOnlyFailure ] This bug is marked as fixed in High Sierra. > LayoutTests/platform/mac/TestExpectations:1391 > # <rdar://problem/30493910> REGRESSION: LayoutTest fast/writing-mode/broken-ideograph-small-caps.html failing This is marked as no longer happening in High Sierra. > LayoutTests/platform/mac/TestExpectations:1394 > # <rdar://problem/30182401> REGRESSION: LayoutTest media/media-source/media-source-abort-resets-parser.html failing This is marked as a duplicate of a fixed bug. > LayoutTests/platform/mac/TestExpectations:1397 > # <rdar://problem/30517283> REGRESSION: LayoutTests security/block-test.html and security/block-test-no-port.html failing This too. There are probably quite a few more, I stopped checking. > LayoutTests/platform/mac/TestExpectations:1425 > # WebCryptoAPI features that enabled only for High Sierra/iOS 11 Surprising that we have to enable these one by one. > LayoutTests/platform/mac/TestExpectations:1635 > +css-dark-mode/older-systems [ Skip ] This directory should be deleted instead. It is surprising that so much unrelated TestExpectations cleanup was done in this patch, without expert review. > LayoutTests/platform/mac/TestExpectations:-1703 > -webkit.org/b/199275 [ HighSierra ] webgpu [ Skip ] > -webkit.org/b/199275 [ HighSierra ] inspector/canvas/create-context-webgpu.html [ Skip ] > -webkit.org/b/199275 [ HighSierra ] inspector/canvas/requestClientNodes-webgpu.html [ Skip ] > -webkit.org/b/199275 [ HighSierra ] inspector/canvas/requestShaderSource-webgpu.html [ Skip ] > -webkit.org/b/199275 [ HighSierra ] inspector/canvas/resolveContext-webgpu.html [ Skip ] > -webkit.org/b/199275 [ HighSierra ] inspector/canvas/shaderProgram-add-remove-webgpu.html [ Skip ] > -webkit.org/b/199275 [ HighSierra ] inspector/canvas/updateShader-webgpu.html [ Skip ] I updated this bug.
What I did in the TestExpectations was expand Sierra and HighSierra to be "false", convert Sierra+, HighSierra+, and Mojave+ to be "true". And make a small number of changes specifically about this patch. No other cleanup or rationalization. That can be done by someone else in another bug.
(In reply to Darin Adler from comment #31) > No other cleanup or rationalization. That can be done by someone else in > another bug. Sorry, that’s wrong. I did a tiny bit more. Adding Pass to tests that are passing.