[MediaRouter] Add implementation of PresentationServiceDelegate

chrome/browser/media/router/presentation_service_delegate_impl.h

Index: chrome/browser/media/router/presentation_service_delegate_impl.h
diff --git a/chrome/browser/media/router/presentation_service_delegate_impl.h b/chrome/browser/media/router/presentation_service_delegate_impl.h
new file mode 100644
index 0000000000000000000000000000000000000000..f5a3e56a56b77b6abce553b05b10a23912fdf2ab
--- /dev/null
+++ b/chrome/browser/media/router/presentation_service_delegate_impl.h
@@ -0,0 +1,212 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.

[mark a. foltz, 2015/05/11 07:36:21]

Update copyright

[haibinlu, 2015/05/13 01:40:20]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_MEDIA_ROUTER_PRESENTATION_SERVICE_DELEGATE_IMPL_H_
+#define CHROME_BROWSER_MEDIA_ROUTER_PRESENTATION_SERVICE_DELEGATE_IMPL_H_
+
+#include <map>
+#include <string>
+#include <utility>
+
+#include "base/gtest_prod_util.h"
+#include "base/macros.h"
+#include "base/memory/linked_ptr.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "base/observer_list.h"
+#include "chrome/browser/media/router/media_router.h"
+#include "chrome/browser/media/router/media_source.h"
+#include "content/public/browser/presentation_service_delegate.h"
+#include "content/public/browser/web_contents_observer.h"
+#include "content/public/browser/web_contents_user_data.h"
+
+namespace content {
+class RenderFrameHost;
+class PresentationScreenAvailabilityListener;
+class WebContents;
+struct PresentationSessionInfo;
+}
+
+namespace media_router {
+
+class MediaRoute;
+class MediaSinksObserver;
+
+// Implementation of PresentationServiceDelegate that interfaces with the

[mark a. foltz, 2015/05/11 07:36:21]

Suggest:

...that interfaces an instance of WebContents with...

and omit the next sentence

[haibinlu, 2015/05/13 01:40:19]

Done.

+// Chrome Media Router. An instance of this class is associated with a
+// WebContents. In addition to fulfilling Presentation requests, it also
+// provide tab-level information that is required for creating
+// browser-initiated sessions.
+class PresentationServiceDelegateImpl
+    : public content::WebContentsUserData<PresentationServiceDelegateImpl>,

[mark a. foltz, 2015/05/11 07:36:22]

This doesn't follow the pattern in WebContentsUserData:

public:
//   virtual ~FooTabHelper();
//   // ... more public stuff here ...
//  private:
//   explicit FooTabHelper(content::WebContents* contents);
//   friend class content::WebContentsUserData<FooTabHelper>;
//   // ... more private stuff here ...
// }

with a private ctor and friend declaration for WCUD.

[haibinlu, 2015/05/13 01:40:20]

Done.

+      public content::PresentationServiceDelegate {
+ public:
+  explicit PresentationServiceDelegateImpl(content::WebContents* web_contents);
+  ~PresentationServiceDelegateImpl() override;
+
+  // content::PresentationServiceDelegate implementation.
+  void AddObserver(
+      int render_process_id,
+      int render_frame_id,
+      content::PresentationServiceDelegate::Observer* observer) override;
+  void RemoveObserver(int render_process_id, int render_frame_id) override;
+  bool AddScreenAvailabilityListener(
+      int render_process_id,
+      int routing_id,
+      content::PresentationScreenAvailabilityListener* listener) override;
+  void RemoveScreenAvailabilityListener(
+      int render_process_id,
+      int routing_id,
+      content::PresentationScreenAvailabilityListener* listener) override;
+  void Reset(int render_process_id, int render_frame_id) override;
+  void SetDefaultPresentationUrl(
+      int render_process_id,
+      int render_frame_id,
+      const std::string& default_presentation_url,
+      const std::string& default_presentation_id) override;
+  void StartSession(int render_process_id,
+                    int render_frame_id,
+                    const std::string& presentation_url,
+                    const std::string& presentation_id,
+                    const PresentationSessionSuccessCallback& success_cb,
+                    const PresentationSessionErrorCallback& error_cb) override;
+  void JoinSession(int render_process_id,
+                   int render_frame_id,
+                   const std::string& presentation_url,
+                   const std::string& presentation_id,
+                   const PresentationSessionSuccessCallback& success_cb,
+                   const PresentationSessionErrorCallback& error_cb) override;
+  void ListenForSessionMessages(
+      int render_process_id,
+      int render_frame_id,
+      const PresentationSessionMessageCallback& message_cb) override;
+
+  // Notifies this object that |route| has been created or joined outside of a

[mark a. foltz, 2015/05/11 07:36:21]

Why does the delegate for a specific webcontents need to be notified about route
creation elsewhere?

[haibinlu, 2015/05/13 01:40:19]

This is for browser initiated sessions.

+  // Presentation API request. The route could be due to
+  // browser action (e.g., browser initiated media router dialog) or
+  // a media route provider (e.g., autojoin).
+  void OnRouteCreated(const MediaRoute& route);
+
+  // Returns the default MediaSource for this tab if there is one.
+  // Returns an empty MediaSource otherwise.
+  MediaSource GetDefaultMediaSource() const;
+
+  // Returns the host name (without the "www.") of the frame that contains

[mark a. foltz, 2015/05/11 07:36:21]

What is this for?  Why do we assume the frame has a host name?

[haibinlu, 2015/05/13 01:40:20]

it is used as domain name as in current extension. Discussed with Derek, he
thinks the domain name should be from a frame that contains default presentation
url. Our current exetnsion behavior is different: we always use the domain of
the tab, e.g., play movie may uses an iframe to load their script from, say,
gstatic.com, the iframe has the presentation URL, but we do not want to show
"cast gstatic.com" because it is confusing to users. 

I sent out an email about this.

+  // the default media source.
+  std::string default_source_host() const { return default_source_host_; }
+
+  const content::WebContents* web_contents() const { return web_contents_; }

[mark a. foltz, 2015/05/11 07:36:21]

Do we need to expose the WebContents in the public API?

[haibinlu, 2015/05/13 01:40:19]

removed

+
+  // Observer interface for listening to default MediaSource changes for the
+  // tab.

[mark a. foltz, 2015/05/11 07:36:22]

s/tab/WebContents/

[haibinlu, 2015/05/13 01:40:19]

Done.

+  class DefaultMediaSourceObserver {
+   public:
+    virtual ~DefaultMediaSourceObserver() {}
+
+    // Called when default media source for the corresponding tab has changed.

[mark a. foltz, 2015/05/11 07:36:22]

s/tab/WebContents/

[haibinlu, 2015/05/13 01:40:19]

Done.

+    // |source|: New default MediaSource, or empty if default was removed.
+    // |source_host|: Host name of the frame that contains the default media
+    // source, or empty if there is no default media source.
+    virtual void OnDefaultMediaSourceChanged(

[mark a. foltz, 2015/05/11 07:36:22]

Do we need to pass a ptr to |this| or WebContents*?  How does the caller know
which WebContents has changed?

[mark a. foltz, 2015/05/11 07:36:22]

It would be a cleaner API to have a separate observer method like
OnDefaultMediaSourceRemoved().

[haibinlu, 2015/05/13 01:40:19]

the call is from the webcontents

+        const MediaSource& source,
+        const std::string& source_host) = 0;
+  };
+
+  // Adds / removes an observer for listening to default MediaSource changes.
+  void AddDefaultMediaSourceObserver(DefaultMediaSourceObserver* observer);

[mark a. foltz, 2015/05/11 07:36:21]

How are these observers different from the AddObserver/RemoveObserver above?

Can we combine the APIs into a single observer interface (unless there are truly
distinct use cases)?

[haibinlu, 2015/05/13 01:40:20]

the observer above is one per WebContents, for WebContents to get sessions
started by browser.

This is for MR to register listener, so that it gets informed about default
presentation url change.

+  void RemoveDefaultMediaSourceObserver(DefaultMediaSourceObserver* observer);
+
+  base::WeakPtr<PresentationServiceDelegateImpl> GetWeakPtr();
+
+ private:
+  friend class PresentationServiceDelegateImplTest;
+  friend class PresentationServiceDelegateImplBrowserTest;
+  FRIEND_TEST_ALL_PREFIXES(PresentationServiceDelegateImplTest,
+                           AddScreenAvailabilityListener);
+  FRIEND_TEST_ALL_PREFIXES(PresentationServiceDelegateImplTest,
+                           MaxNumObservers);
+  FRIEND_TEST_ALL_PREFIXES(PresentationServiceDelegateImplTest,
+                           AddListenerSameUrlTwice);
+  FRIEND_TEST_ALL_PREFIXES(PresentationServiceDelegateImplTest, Reset);
+  FRIEND_TEST_ALL_PREFIXES(PresentationServiceDelegateImplBrowserTest,
+                           RemoveObserversOnNavigation);
+
+  typedef base::hash_map<MediaSourceId, linked_ptr<MediaSinksObserver>>
+      MediaSinksObserverBySource;
+
+  // TODO(imcheng): Move this to PresentationServiceDelegate and use it in the

[mark a. foltz, 2015/05/11 07:36:22]

Can we implement this TODO now?

[haibinlu, 2015/05/13 01:40:20]

Done.

+  // API.
+  typedef std::pair<int, int> RenderFrameHostId;

[mark a. foltz, 2015/05/11 07:36:22]

Class members need to be declared after methods and functions.

http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Declaration_O...

[haibinlu, 2015/05/13 01:40:21]

Acknowledged.

+
+  // Maximum number of sources that can be registered at a time.
+  static const size_t kMaxNumSources = 256;
+
+  // Helper function to extract ID for a RFH.
+  static RenderFrameHostId GetRenderFrameHostId(content::RenderFrameHost* rfh);

[mark a. foltz, 2015/05/11 07:36:22]

I usually prefer to declare helper functions for the .cc in an anonymous
namespace there, instead of  private static members of the class.  This keeps
the .h smaller and the class declaration free of helper function clutter.

[haibinlu, 2015/05/13 01:40:21]

Done.

+
+  // Helper function to unregister and remove all MediaSinksObservers that
+  // are associated with |rfh_id|.
+  void UnregisterAllObserversFromFrame(RenderFrameHostId rfh_id);
+
+  // Returns |listener|'s presentation URL as a MediaSource. If |listener| does
+  // not have a persentation URL, default to tab mirroring.
+  MediaSource GetMediaSourceFromListener(
+      content::PresentationScreenAvailabilityListener* listener);
+
+  // Gets the host name associated with the frame given by |rfh_id|.
+  std::string GetSourceHostForFrame(RenderFrameHostId rfh_id) const;

[mark a. foltz, 2015/05/11 07:36:22]

Does this need to be an instance method or can it be a helper function in the
.cc?

[haibinlu, 2015/05/13 01:40:20]

No. move to anonymous namespace.

+
+  // Returns the presentation ID to use for frame |rfh_id|.
+  // If |presentation_id| is non-empty, return it.
+  // Otherwise, check if a default presentation ID is set for that frame, and
+  // return it.
+  // Otherwise, returns a randomly generated string.
+  std::string GetOrGeneratePresentationId(

[mark a. foltz, 2015/05/11 07:36:21]

Ditto

[haibinlu, 2015/05/13 01:40:20]

need to access default_presentation_infos_

+      RenderFrameHostId rfh_id,
+      const std::string& presentation_id) const;
+
+  // Returns |true| is frame |rfh_id| is the main frame of |web_contents_|.
+  bool IsMainFrame(RenderFrameHostId rfh_id) const;

[mark a. foltz, 2015/05/11 07:36:22]

Ditto

[haibinlu, 2015/05/13 01:40:20]

Need to access web_contents_

+
+  // Updates tab-level default MediaSource and source host name. If either
+  // changed, notify the observers.
+  void UpdateDefaultMediaSourceAndNotifyObservers(
+      const MediaSource& new_default_source,
+      const std::string& new_default_source_host);
+
+  void SetMediaRouterForTest(MediaRouter* router);
+
+  // Maps from RenderFrameHost ID to MediaSinksObservers for that frame.
+  base::hash_map<RenderFrameHostId, MediaSinksObserverBySource> observers_;
+  size_t num_observers_;
+
+  // Maps from RFH ID to default presentation.
+  base::hash_map<RenderFrameHostId,
+                 linked_ptr<content::PresentationSessionInfo>>
+      default_presentation_infos_;
+
+  MediaSource default_source_;
+  std::string default_source_host_;
+
+  content::WebContents* web_contents_;

[mark a. foltz, 2015/05/11 07:36:21]

This is unowned correct?

[haibinlu, 2015/05/13 01:40:20]

correct.

+
+  // Observers listening for changes to this class. Not owned by this class.
+  base::hash_map<RenderFrameHostId, Observer*> delegate_observers_;

[mark a. foltz, 2015/05/11 07:36:21]

I'm confused here, why isn't this an ObserverList?

[haibinlu, 2015/05/13 01:40:19]

Need to index it by rfh_id. linear list works too.

+
+  // Observers listening for changes to default media source. Not owned by
+  // this class.
+  ObserverList<DefaultMediaSourceObserver> default_media_source_observers_;
+
+  // Does not own this object.
+  MediaRouter* router_;
+
+  // NOTE: All WeakPtrs must be invalidated before member variables.
+  base::WeakPtrFactory<PresentationServiceDelegateImpl> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(PresentationServiceDelegateImpl);
+};
+
+}  // namespace media_router
+
+#endif  // CHROME_BROWSER_MEDIA_ROUTER_PRESENTATION_SERVICE_DELEGATE_IMPL_H_