[chrome.displaySource] Use WDS for Wi-Fi Display implementation

extensions/renderer/api/display_source/wifi_display/wifi_display_session.h

 // Copyright 2015 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_RENDERER_API_DISPLAY_SOURCE_WIFI_DISPLAY_WIFI_DISPLAY_SESSION_H_
 #define EXTENSIONS_RENDERER_API_DISPLAY_SOURCE_WIFI_DISPLAY_WIFI_DISPLAY_SESSION_H_
 
+#include <map>
 #include <string>
 
 #include "extensions/common/mojo/wifi_display_session_service.mojom.h"
 #include "extensions/renderer/api/display_source/display_source_session.h"
 #include "mojo/public/cpp/bindings/binding.h"
+#include "third_party/wds/src/libwds/public/source.h"
+
+namespace base {
+class Timer;
+}  // namespace base
 
 namespace extensions {
 
+class WiFiDisplayMediaManager;
+
 class WiFiDisplaySession: public DisplaySourceSession,

[asargent_no_longer_on_chrome, 2016/02/25 22:09:39]

nit: this class could use a brief overview comment describing its overall role

[Mikhail, 2016/02/26 14:42:46]

Done.

-                          public WiFiDisplaySessionServiceClient {
+                          public WiFiDisplaySessionServiceClient,
+                          public wds::Peer::Delegate,
+                          public wds::Peer::Observer {
  public:
   explicit WiFiDisplaySession(
       const DisplaySourceSessionParams& params);
   ~WiFiDisplaySession() override;
 
  private:
   // DisplaySourceSession overrides.
   void Start() override;
   void Terminate() override;
 
   // WiFiDisplaySessionServiceClient overrides.
   void OnEstablished(const mojo::String& ip_address) override;
   void OnTerminated() override;
   void OnError(int32_t type, const mojo::String& description) override;
   void OnMessage(const mojo::String& data) override;
 
+  // wds::Peer::Delegate overrides.
+  unsigned CreateTimer(int seconds) override;
+  void ReleaseTimer(unsigned timer_id) override;
+  void SendRTSPData(const std::string& message) override;
+  std::string GetLocalIPAddress() const override;
+  int GetNextCSeq(int* initial_peer_cseq = nullptr) const override;
+
+  // wds::Peer::Observer overrides.
+  void ErrorOccurred(wds::ErrorType error) override;
+  void SessionCompleted() override;
+
   // A connection error handler for the mojo objects used in this class.
-  void OnConnectionError();
+  void OnIPCConnectionError();
 
  private:
+  scoped_ptr<wds::Source> wfd_source_;
+  scoped_ptr<WiFiDisplayMediaManager> media_manager_;
   WiFiDisplaySessionServicePtr service_;
   mojo::Binding<WiFiDisplaySessionServiceClient> binding_;
   std::string ip_address_;
+  std::map<unsigned, scoped_ptr<base::Timer>> timers_;
+
   DisplaySourceSessionParams params_;
+  mutable int cseq_;

[asargent_no_longer_on_chrome, 2016/02/25 22:09:39]

nits: 
-"cseq" isn't a very descriptive name - at a minimum this needs a descriptive
comment describing what it is, and if it's an acronym it's probably better to
use the complete name as it's not a very commonly known acronym. 
-do you really need to use mutable here? It's use is allowed but somewhat
discouraged due to thread safety issues. 

[Mikhail, 2016/02/26 14:42:46]

Added a description comment. This member is mutable because it is modified in a
const method.

[asargent_no_longer_on_chrome, 2016/02/26 17:01:34]

To be clear, the point I was trying to make is as follows: Does the modification
of cseq change the externally visible state of the object? If so the preferred
solution is to use non-const ref/ptr wherever you need to call a method that
results in cseq changing, and make that method non-const. I'd however cseq is
something like a cache that gets lazily populated on an otherwise const method,
then using mutable is appropriate. 

+  unsigned timer_id_;

[asargent_no_longer_on_chrome, 2016/02/25 22:09:39]

nit: we usually prefer to use either int, size_t, or one of the specific integer
type definitions, and actively discourage using unsigned integers to represent
"integers that should be non-negative"; see
https://google.github.io/styleguide/cppguide.html#Integer_Types

[Mikhail, 2016/02/26 14:42:46]

'unsigned' was used due to the inherited WDS interface, but I believe it should
be fixed in WDS (will switch interface to using 'int' type in the following WDS
version), thanks.

   base::WeakPtrFactory<WiFiDisplaySession> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(WiFiDisplaySession);
 };
 
 }  // namespace extensions
 
 #endif  // EXTENSIONS_RENDERER_API_DISPLAY_SOURCE_WIFI_DISPLAY_WIFI_DISPLAY_SESSION_H_