Bug 223730 - NowPlayingInfoArtwork operator== returns wrong value
Summary: NowPlayingInfoArtwork operator== returns wrong value
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jean-Yves Avenard [:jya]
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-24 22:32 PDT by Jean-Yves Avenard [:jya]
Modified: 2021-03-26 04:07 PDT (History)
11 users (show)

See Also:

Attachments
Patch (1.35 KB, patch)
2021-03-25 02:16 PDT, Jean-Yves Avenard [:jya] 		no flags Details | Formatted Diff | Diff
Patch (9.83 KB, patch)
2021-03-25 16:14 PDT, Jean-Yves Avenard [:jya] 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (9.83 KB, patch)
2021-03-25 16:22 PDT, Jean-Yves Avenard [:jya] 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jean-Yves Avenard [:jya] 2021-03-24 22:32:17 PDT 
Bug introduced in bug 223118

NowPlayingInfoArtwork::operator== checks if the value differs.
Comment 1 Radar WebKit Bug Importer 2021-03-24 22:32:26 PDT 
<rdar://problem/75822687>
Comment 2 Jean-Yves Avenard [:jya] 2021-03-25 02:16:15 PDT 
Created attachment 424224 [details]
Patch
Comment 3 youenn fablet 2021-03-25 07:20:27 PDT 
Comment on attachment 424224 [details]
Patch

I would be tempted to change the patch so that we define == operator.
And implement != operator based on ==
Comment 4 youenn fablet 2021-03-25 07:20:47 PDT 
Comment on attachment 424224 [details]
Patch

r=me
Comment 5 youenn fablet 2021-03-25 07:27:46 PDT 
Rethinking about it, we should probably add a test for it.
The principle is to write a unit test (or API test).
You can for instance look at examples in Tools/TestWebKitAPI/Tests/WebCore/IntRectTests.cpp.
Comment 6 Jean-Yves Avenard [:jya] 2021-03-25 16:14:47 PDT 
Created attachment 424292 [details]
Patch
Comment 7 Jean-Yves Avenard [:jya] 2021-03-25 16:22:58 PDT 
Created attachment 424293 [details]
Patch
Comment 8 youenn fablet 2021-03-26 03:48:48 PDT 
Comment on attachment 424293 [details]
Patch

Landing the changes now.
Some additional comments below.

View in context: https://bugs.webkit.org/attachment.cgi?id=424293&action=review

> Source/WebCore/platform/audio/NowPlayingInfo.h:42
>      bool operator==(const NowPlayingInfoArtwork& other) const

Given this is a struct, we could make it a free function, which is usually preferred when possible.

> Tools/TestWebKitAPI/Tests/WebCore/NowPlayingInfoTests.cpp:32
> +static void testEmptyArtwork(const WebCore::NowPlayingInfoArtwork& artwork)

Not really needed, it could be inline in the TEST itself.
Comment 9 EWS 2021-03-26 04:07:50 PDT 
Committed r275091: <https://commits.webkit.org/r275091>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424293 [details].