Fix PlayerIdentifier::operator<()

PlayerIdentifier::operator<() was defined through operator<() on
PlayerIdentifier hashes.  This almost always gives correct results, but
not in general.  Hash collisions are possible (collisions of sums of
hashes are a bit "more possible", see old PlayerIdentifier::Hash()), so
operator<() could return false for two PlayerIdentifier instances that
are actually in a less-than relationship.

The hash function is not really needed anyway, because base::flat_set is
better suited here than std::unordered_set.

Change-Id: I12eec1aa27774d0bd6e10232c35968832fed1ff7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2492440Reviewed-by: Becca Hughes <beccahughes@chromium.org>
Commit-Queue: Wojciech Dzierżanowski <wdzierzanowski@opera.com>
Cr-Commit-Position: refs/heads/master@{#823641}

content/browser/media/session/media_session_impl.cc

@@ -174,16 +174,8 @@ bool MediaSessionImpl::PlayerIdentifier::operator==(
 
 bool MediaSessionImpl::PlayerIdentifier::operator<(
     const PlayerIdentifier& other) const {
-  return MediaSessionImpl::PlayerIdentifier::Hash()(*this) <
-         MediaSessionImpl::PlayerIdentifier::Hash()(other);
-}
-
-size_t MediaSessionImpl::PlayerIdentifier::Hash::operator()(
-    const PlayerIdentifier& player_identifier) const {
-  size_t hash =
-      std::hash<MediaSessionPlayerObserver*>()(player_identifier.observer);
-  hash += std::hash<int>()(player_identifier.player_id);
-  return hash;
+  return observer != other.observer ? observer < other.observer
+                                    : player_id < other.player_id;
 }
 
 // static
@@ -421,19 +413,10 @@ bool MediaSessionImpl::AddPlayer(MediaSessionPlayerObserver* observer,
 
 void MediaSessionImpl::RemovePlayer(MediaSessionPlayerObserver* observer,
                                     int player_id) {
-  PlayerIdentifier identifier(observer, player_id);
-
-  auto iter = normal_players_.find(identifier);
-  if (iter != normal_players_.end())
-    normal_players_.erase(iter);
-
-  auto it = pepper_players_.find(identifier);
-  if (it != pepper_players_.end())
-    pepper_players_.erase(it);
-
-  it = one_shot_players_.find(identifier);
-  if (it != one_shot_players_.end())
-    one_shot_players_.erase(it);
+  const PlayerIdentifier identifier(observer, player_id);
+  normal_players_.erase(identifier);
+  pepper_players_.erase(identifier);
+  one_shot_players_.erase(identifier);
 
   AbandonSystemAudioFocusIfNeeded();
   UpdateRoutedService();

content/browser/media/session/media_session_impl.h

@@ -9,11 +9,10 @@
 
 #include <map>
 #include <set>
-#include <unordered_map>
-#include <unordered_set>
 #include <vector>
 
 #include "base/containers/flat_map.h"
+#include "base/containers/flat_set.h"
 #include "base/containers/id_map.h"
 #include "base/macros.h"
 #include "base/optional.h"
@@ -314,22 +313,19 @@ class MediaSessionImpl : public MediaSession,
   // Representation of a player for the MediaSessionImpl.
   struct PlayerIdentifier {
     PlayerIdentifier(MediaSessionPlayerObserver* observer, int player_id);
+
     PlayerIdentifier(const PlayerIdentifier&) = default;
+    PlayerIdentifier(PlayerIdentifier&&) = default;
 
-    void operator=(const PlayerIdentifier&) = delete;
-    bool operator==(const PlayerIdentifier& player_identifier) const;
-    bool operator<(const PlayerIdentifier&) const;
+    PlayerIdentifier& operator=(const PlayerIdentifier&) = default;
+    PlayerIdentifier& operator=(PlayerIdentifier&&) = default;
 
-    // Hash operator for std::unordered_map<>.
-    struct Hash {
-      size_t operator()(const PlayerIdentifier& player_identifier) const;
-    };
+    bool operator==(const PlayerIdentifier& other) const;
+    bool operator<(const PlayerIdentifier& other) const;
 
     MediaSessionPlayerObserver* observer;
     int player_id;
   };
-  using PlayersMap =
-      std::unordered_set<PlayerIdentifier, PlayerIdentifier::Hash>;
 
   CONTENT_EXPORT explicit MediaSessionImpl(WebContents* web_contents);
 
@@ -430,11 +426,11 @@ class MediaSessionImpl : public MediaSession,
   std::unique_ptr<AudioFocusDelegate> delegate_;
   std::map<PlayerIdentifier, media_session::mojom::AudioFocusType>
       normal_players_;
-  PlayersMap pepper_players_;
+  base::flat_set<PlayerIdentifier> pepper_players_;
 
   // Players that are playing in the web contents but we cannot control (e.g.
   // WebAudio or MediaStream).
-  PlayersMap one_shot_players_;
+  base::flat_set<PlayerIdentifier> one_shot_players_;
 
   State audio_focus_state_ = State::INACTIVE;
   MediaSession::SuspendType suspend_type_;