Bugs like: https://bugs.webkit.org/show_bug.cgi?id=222277 are making this directory very difficult to work around. These tests are causing issues on EWS and CQ. Jer proposed we skip all these tests while he works on a workaround.
<rdar://problem/75492360>
I am marking the whole directory as [ Pass Failure ] for Mac which will skip these all on EWS but keep them running on the production bots.
Committed: https://trac.webkit.org/changeset/274511/webkit
Created attachment 423407 [details] Patch
Comment on attachment 423407 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423407&action=review > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:846 > + size_t count = 0; > + auto results = fragmentParameter.splitAllowingEmptyEntries('='); > + for (auto iterator = results.begin(); iterator != results.end(); ++iterator) > + ++count; > + return count != 2; Couldn't this be just: return fragmentParameter.splitAllowingEmptyEntries('=').size != 2;
Comment on attachment 423407 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423407&action=review >> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:846 >> + return count != 2; > > Couldn't this be just: > > return fragmentParameter.splitAllowingEmptyEntries('=').size != 2; This seems a roundabout way to check if to be sure there is exactly one "=". I think that’s what it means to split allowing empty and then count and then check that there are exactly 2. Unnecessary to count all the "=" signs to check if there are more than one; ideally we’d stop once the count hits 2. But I’ve never found we have a great idiom for that in cases like this. I think I would write this: auto hasInvalidNumberOfEqualCharacters = [] (StringView fragmentParameter) { unsigned count = 0; for (auto character : fragmentParameter.codeUnits()) { if (character == '=') { if (++count > 1) return true; } } return count != 1; } But it is sad that this function is so long!
(In reply to Eric Carlson from comment #5) > Comment on attachment 423407 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=423407&action=review > > Couldn't this be just: > > return fragmentParameter.splitAllowingEmptyEntries('=').size != 2; No, because a SplitResult is just an iterator pair; and the iterators aren't random access, so they can't be subtracted from one another.
(In reply to Darin Adler from comment #6) > Comment on attachment 423407 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=423407&action=review > > >> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:846 > >> + return count != 2; > > > > Couldn't this be just: > > > > return fragmentParameter.splitAllowingEmptyEntries('=').size != 2; > > This seems a roundabout way to check if to be sure there is exactly one "=". > I think that’s what it means to split allowing empty and then count and then > check that there are exactly 2. > > Unnecessary to count all the "=" signs to check if there are more than one; > ideally we’d stop once the count hits 2. But I’ve never found we have a > great idiom for that in cases like this. > > I think I would write this: > > auto hasInvalidNumberOfEqualCharacters = [] (StringView > fragmentParameter) { > unsigned count = 0; > for (auto character : fragmentParameter.codeUnits()) { > if (character == '=') { > if (++count > 1) > return true; > } > } > return count != 1; > } > > But it is sad that this function is so long! So I avoided this solution exactly because it was too long. But because a StringResult iterator basically encapsulates that search algorithm above, I think you could get the same benefit just by re-writing the `hasInvalidNumberOfEqualCharacters` lambda: auto hasInvalidNumberOfEqualCharacters = [](StringView& fragmentParameter) { size_t count = 0; auto results = fragmentParameter.splitAllowingEmptyEntries('='); for (auto iterator = results.begin(); iterator != results.end(); ++iterator) { if (++count > 1) return false; } return count != 1; }; Actually, you may be able to do something like: auto hasInvalidNumberOfEqualCharacters = [](StringView& fragmentParameter) { auto results = fragmentParameter.splitAllowingEmptyEntries('='); auto iterator = results.begin(); return iterator == results.end() || ++iterator == results.end() || ++iterator != results.end(); } Not sure that the latter is better just because it's shorter though.
Comment on attachment 423407 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423407&action=review >>>> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:846 >>>> + return count != 2; >>> >>> Couldn't this be just: >>> >>> return fragmentParameter.splitAllowingEmptyEntries('=').size != 2; >> >> This seems a roundabout way to check if to be sure there is exactly one "=". I think that’s what it means to split allowing empty and then count and then check that there are exactly 2. >> >> Unnecessary to count all the "=" signs to check if there are more than one; ideally we’d stop once the count hits 2. But I’ve never found we have a great idiom for that in cases like this. >> >> I think I would write this: >> >> auto hasInvalidNumberOfEqualCharacters = [] (StringView fragmentParameter) { >> unsigned count = 0; >> for (auto character : fragmentParameter.codeUnits()) { >> if (character == '=') { >> if (++count > 1) >> return true; >> } >> } >> return count != 1; >> } >> >> But it is sad that this function is so long! > > So I avoided this solution exactly because it was too long. But because a StringResult iterator basically encapsulates that search algorithm above, I think you could get the same benefit just by re-writing the `hasInvalidNumberOfEqualCharacters` lambda: > > auto hasInvalidNumberOfEqualCharacters = [](StringView& fragmentParameter) { > size_t count = 0; > auto results = fragmentParameter.splitAllowingEmptyEntries('='); > for (auto iterator = results.begin(); iterator != results.end(); ++iterator) { > if (++count > 1) > return false; > } > return count != 1; > }; > > Actually, you may be able to do something like: > > auto hasInvalidNumberOfEqualCharacters = [](StringView& fragmentParameter) { > auto results = fragmentParameter.splitAllowingEmptyEntries('='); > auto iterator = results.begin(); > return iterator == results.end() || ++iterator == results.end() || ++iterator != results.end(); > } > > Not sure that the latter is better just because it's shorter though. One peculiar thing is the & on the StringView&. I like that last one.
Created attachment 423634 [details] Patch for landing
Created attachment 423645 [details] Patch for landing
contributors.json is malformed: Expecting ',' delimiter: line 626 column 16 (char 13043)
commit-queue failed to commit attachment 423645 [details] to WebKit repository. To retry, please set cq+ flag again.
Committed r274734: <https://commits.webkit.org/r274734> All reviewed patches have been landed. Closing bug and clearing flags on attachment 423645 [details].