[Presentation API] Add support for multiple URLs in PresentationRequest on Media Router UI side

chrome/browser/ui/webui/media_router/cast_modes_with_media_sources.h

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_UI_WEBUI_MEDIA_ROUTER_CAST_MODES_WITH_MEDIA_SOURCES_H_
+#define CHROME_BROWSER_UI_WEBUI_MEDIA_ROUTER_CAST_MODES_WITH_MEDIA_SOURCES_H_
+
+#include <map>
+#include <unordered_set>
+
+#include "chrome/browser/media/router/media_source.h"
+#include "chrome/browser/ui/webui/media_router/media_cast_mode.h"
+
+namespace media_router {
+
+// Contains information on cast modes and the sources associated with them.
+// Each cast mode contained has at least one source.
+class CastModesWithMediaSources {
+ public:
+  CastModesWithMediaSources();
+  CastModesWithMediaSources(const CastModesWithMediaSources& other);

[imcheng, 2016/09/12 19:26:28]

This object looks a bit expensive to copy. Is copy ctor really needed, or can we
disallow copy assignment/construction instead?

[takumif, 2016/09/13 03:48:21]

It's needed for implicitly instantiating with the [] operator for unordered_map
in QueryResultManager.

[mark a. foltz, 2016/09/13 17:30:15]

I usually code to avoid the [] operator since it results in the side effect of
instantiation with lookup, and map[foo] = bar() results in both calling the
default ctor, the ctor for bar(), and the assignment operator.

In this case, I am also not sure it makes sense to copy a CMWMS: there should be
only one instance in |cast_mode_sources_| per MediaSource.

I recommend:
- Disallowing copy and assign.
- Changing the unordered_map to store unique_ptr *or* declare a move ctor
(slight preference for latter).

References:
https://google.github.io/styleguide/cppguide.html#Copyable_Movable_Types
https://google.github.io/styleguide/cppguide.html#Rvalue_references

[takumif, 2016/09/13 19:20:34]

Got it. Declaring a move ctor instead.

+  ~CastModesWithMediaSources();
+
+  // Add a source for the cast mode.

[mark a. foltz, 2016/09/09 22:22:26]

Super nit: Method comments should begin "Adds", "Removes", etc.

[takumif, 2016/09/13 03:48:21]

Done.

+  void AddSource(MediaCastMode cast_mode, const MediaSource& source);
+
+  // Remove a source from the cast mode. The cast mode will also get removed if
+  // it has no other sources. This is a no-op if the source is not found.
+  void RemoveSource(MediaCastMode cast_mode, const MediaSource& source);
+
+  // Returns true if the source for the cast mode is contained.
+  bool HasSource(MediaCastMode cast_mode, const MediaSource& source) const;
+
+  // Returns a set of all the cast modes contained.
+  CastModeSet GetCastModes() const;
+
+  // Returns true if there are no cast modes contained.
+  bool IsEmpty() const;
+
+ private:
+  std::map<MediaCastMode, std::unordered_set<MediaSource, MediaSource::Hash>>
+  cast_modes_;

[mark a. foltz, 2016/09/09 22:22:26]

Indentation

[takumif, 2016/09/13 03:48:21]

Done.

+};
+
+}  // namespace media_router
+
+#endif  // CHROME_BROWSER_UI_WEBUI_MEDIA_ROUTER_CAST_MODES_WITH_MEDIA_SOURCES_H_