[cast_channel] Make CastSocket not inherit from ApiResource

extensions/browser/api/cast_channel/cast_channel_service.h

+// Copyright 2017 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 EXTENSIONS_BROWSER_API_CAST_CHANNEL_CAST_CHANNEL_SERVICE_H_
+#define EXTENSIONS_BROWSER_API_CAST_CHANNEL_CAST_CHANNEL_SERVICE_H_
+
+#include <map>
+#include <memory>
+
+#include "base/macros.h"
+#include "base/threading/thread_checker.h"
+#include "components/keyed_service/core/keyed_service.h"
+#include "content/public/browser/browser_thread.h"
+#include "extensions/browser/api/cast_channel/cast_socket.h"
+
+namespace extensions {
+namespace api {
+namespace cast_channel {
+
+// This class manages cast sockets created by CastChannelAPI.

[mark a. foltz, 2017/05/18 18:02:26]

Does this comment belong with CastChannelService?

[zhaobin, 2017/05/24 01:51:40]

CastSocketRegistry does the actual management...Modified a little.

+// This class is not thread safe. All methods must be called from the IO thread.
+class CastSocketData {

[mark a. foltz, 2017/05/18 18:02:26]

Bikeshed: CastSocketRegistry?

[zhaobin, 2017/05/24 01:51:40]

Done.

+ public:
+  CastSocketData();
+  ~CastSocketData();
+
+  // Adds |socket| to |manager_| and returns the new channel_id. Takes ownership

[mark a. foltz, 2017/05/18 18:02:26]

Where is |manager_| declared?

[zhaobin, 2017/05/24 01:51:40]

Done.

+  // of |socket|.
+  int AddSocket(CastSocket* socket);

[mark a. foltz, 2017/05/18 18:02:26]

When ownership is transferred via parameter, it preferable to use a moveable
type, in this case std::unique_ptr<CastSocket>

[zhaobin, 2017/05/24 01:51:39]

Done.

+
+  // Removes the CastSocket corresponding to |channel_id| from the resource
+  // manager.
+  void RemoveSocket(int channel_id);

[mark a. foltz, 2017/05/18 18:02:26]

Nit: Consider returning std::unique_ptr<CastSocket> as this class is now
relinquishing ownership.

[zhaobin, 2017/05/24 01:51:39]

Done.

+
+  // Returns the socket corresponding to |channel_id| if one exists, or null
+  // otherwise.
+  CastSocket* GetSocket(int channel_id) const;
+
+ private:
+  // The collection of CastSocket.
+  std::map<int, std::unique_ptr<CastSocket>> sockets_;
+
+  THREAD_CHECKER(thread_checker_);
+
+  DISALLOW_COPY_AND_ASSIGN(CastSocketData);
+};
+
+// This class associates underlying CastSocketData instance with BrowserContext
+// and makes sure CastSocketData instance is created and destroyed on the IO
+// thread.
+class CastChannelService : public KeyedService {

[mark a. foltz, 2017/05/18 18:02:26]

Bikeshed: Slight preference to name this class CastSocketManager, but I don't
feel strongly

[zhaobin, 2017/05/24 01:51:40]

It is a KeyedService, would like to keep Service...CastSocketService?

[mark a. foltz, 2017/05/24 17:44:10]

Yeah that makes sense.

+ public:
+  CastChannelService();
+  ~CastChannelService() override;
+
+  // Creates CastSocketData instance if none exists.
+  CastSocketData* GetOrCreateSocketData();

[mark a. foltz, 2017/05/18 18:02:26]

What thread is expected to invoke this method?

[zhaobin, 2017/05/24 01:51:40]

Done.

+
+ private:
+  std::unique_ptr<CastSocketData, content::BrowserThread::DeleteOnIOThread>
+      sockets_;
+
+  DISALLOW_COPY_AND_ASSIGN(CastChannelService);
+};
+
+}  // namespace cast_channel
+}  // namespace api
+}  // namespace extensions
+
+#endif  // EXTENSIONS_BROWSER_API_CAST_CHANNEL_CAST_CHANNEL_SERVICE_H_