[Chromecast] Turn CastContentWindow into an abstract interface.

chromecast/browser/cast_content_window.h

Index: chromecast/browser/cast_content_window.h
diff --git a/chromecast/browser/cast_content_window.h b/chromecast/browser/cast_content_window.h
index aeeeab94f021cb51ab83195e41442e277ebb16e0..7ec1d69d35a1d5ae6ca21b422a75efaa26dc6821 100644
--- a/chromecast/browser/cast_content_window.h
+++ b/chromecast/browser/cast_content_window.h
@@ -8,12 +8,7 @@
 #include <memory>
 
 #include "base/macros.h"
-#include "chromecast/graphics/cast_vsync_settings.h"
-#include "content/public/browser/web_contents_observer.h"
-
-namespace aura {
-class WindowTreeHost;
-}
+#include "ui/events/keycodes/keyboard_codes.h"
 
 namespace content {
 class BrowserContext;
@@ -23,42 +18,38 @@ class WebContents;
 namespace chromecast {
 namespace shell {
 
-class CastContentWindow : public content::WebContentsObserver,
-                          public CastVSyncSettings::Observer {
+// Class that represents the "window" a WebContents is displayed in cast_shell.
+// For Linux, this represents an Aura window. For Android, this is a Activity.
+// See CastContentWindowLinux and CastContentWindowAndroid.
+class CastContentWindow {
  public:
-  CastContentWindow();
-
-  // Removes the window from the screen.
-  ~CastContentWindow() override;
-
-  // Sets the window's background to be transparent (call before
-  // CreateWindowTree).
-  void SetTransparent() { transparent_ = true; }
+  class Delegate {
+   public:
+    virtual void OnWindowDestroyed() = 0;
+    virtual void OnKeyDown(ui::KeyboardCode keycode) = 0;

[Wez, 2017/01/11 20:47:54]

KeyboardCode is deprecated (though it seems we still use it under Android, and
don't generate the newer |key| and |code| values on events ):

I'd suggest having this interface be OnKeyEvent(const ui::KeyEvent&), so that
it's not tied to the deprecated keyCode field, even though the receiving code
will need to use that field for now, in the absence of |key| and |code|.

[derekjchow1, 2017/01/11 21:54:33]

SGTM. Done.

Unfortunately, this changes our new dependency from ui/events/keycodes to
ui/events (meaning we need a new OWNERS reviewer for approval). +sadrul

 
-  // Create a full-screen window for |web_contents|.
-  void CreateWindowTree(content::WebContents* web_contents);
+   protected:
+    virtual ~Delegate() {}
+  };
 
-  std::unique_ptr<content::WebContents> CreateWebContents(
-      content::BrowserContext* browser_context);
+  // Creates the platform specific CastContentWindow.
+  static std::unique_ptr<CastContentWindow> Create(
+      CastContentWindow::Delegate* delegate);

[Wez, 2017/01/11 20:47:54]

nit: Please be specific about the lifetime requirement on |delegate|, e.g. I
assume it must out-live CastContentWindow?

Is it beneficial to have |delegate| be a non-owned interface like this, or could
it be an owned interface, i.e. the Create() call would receive a
unique_ptr<Delegate> rather than bare-pointer?

[derekjchow1, 2017/01/11 21:54:34]

Added comment about |delegate| outliving CastContentWindow.

The delegate interface should be non-owning. Internally, the parent object of
CastContentWindow will pass itself as the delegate to be notified when the
window is destroyed or keys are pressed.

[Wez, 2017/01/12 21:04:28]

Acknowledged.

 
-  // content::WebContentsObserver implementation:
-  void DidFirstVisuallyNonEmptyPaint() override;
-  void MediaStartedPlaying(const MediaPlayerInfo& media_info,
-                           const MediaPlayerId& id) override;
-  void MediaStoppedPlaying(const MediaPlayerInfo& media_info,
-                           const MediaPlayerId& id) override;
-  void RenderViewCreated(content::RenderViewHost* render_view_host) override;
+  virtual ~CastContentWindow() {}
 
-  // CastVSyncSettings::Observer implementation:
-  void OnVSyncIntervalChanged(base::TimeDelta interval) override;
+  // Sets the window's background to be transparent (call before
+  // CreateWindowTree).
+  virtual void SetTransparent() = 0;
 
- private:
-#if defined(USE_AURA)
-  std::unique_ptr<aura::WindowTreeHost> window_tree_host_;
-#endif
-  bool transparent_;
+  // Creates a full-screen window for |web_contents| and display it.
+  virtual void ShowWebContents(content::WebContents* web_contents) = 0;

[Wez, 2017/01/11 20:47:54]

Similarly, please clarify the lifetime guarantee on |web_contents| that is
expected of the caller.

[derekjchow1, 2017/01/11 21:54:34]

Done.

 
-  DISALLOW_COPY_AND_ASSIGN(CastContentWindow);
+  // Creates a WebContents.
+  // TODO(derekjchow): remove this function from this class, since it doesn't
+  // have anything to do with displaying web_contents.
+  virtual std::unique_ptr<content::WebContents> CreateWebContents(
+      content::BrowserContext* browser_context) = 0;
 };
 
 }  // namespace shell