Nukes NativeViewportClient::OnCreated

And replaces with a callback. Also renames SetBounds to SetSize.

BUG=none
TEST=none
R=ben@chromium.org

Review URL: https://codereview.chromium.org/607233002

Cr-Commit-Position: refs/heads/master@{#297187}

mojo/examples/compositor_app/compositor_app.cc

@@ -5,7 +5,9 @@
 #include <stdio.h>
 #include <string>
 
+#include "base/bind.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "mojo/application/application_runner_chromium.h"
 #include "mojo/examples/compositor_app/compositor_host.h"
 #include "mojo/public/c/system/main.h"
@@ -22,32 +24,26 @@ namespace examples {
 
 class SampleApp : public ApplicationDelegate, public NativeViewportClient {
  public:
-  SampleApp() {}
+  SampleApp() : weak_factory_(this) {}
   virtual ~SampleApp() {}
 
   virtual void Initialize(ApplicationImpl* app) OVERRIDE {
     app->ConnectToService("mojo:mojo_native_viewport_service", &viewport_);
     viewport_.set_client(this);
-    viewport_->Create(Size::From(gfx::Size(800, 600)));
+    viewport_->Create(Size::From(gfx::Size(800, 600)),
+                      base::Bind(&SampleApp::OnCreatedNativeViewport,
+                                 weak_factory_.GetWeakPtr()));
     viewport_->Show();
 
     // TODO(jamesr): Should be mojo:mojo_gpu_service
     app->ConnectToService("mojo:mojo_native_viewport_service", &gpu_service_);
   }
 
-  virtual void OnCreated(uint64_t native_viewport_id) OVERRIDE {
-    CommandBufferPtr cb;
-    // TODO(jamesr): Output to a surface instead.
-    gpu_service_->CreateOnscreenGLES2Context(
-        native_viewport_id, Size::From(gfx::Size(800, 600)), Get(&cb));
-    host_.reset(new CompositorHost(cb.PassMessagePipe()));
-  }
-
   virtual void OnDestroyed() OVERRIDE { base::MessageLoop::current()->Quit(); }
 
-  virtual void OnBoundsChanged(SizePtr bounds) OVERRIDE {
+  virtual void OnSizeChanged(SizePtr size) OVERRIDE {
     if (host_)
-      host_->SetSize(bounds.To<gfx::Size>());
+      host_->SetSize(size.To<gfx::Size>());
   }
 
   virtual void OnEvent(EventPtr event,
@@ -56,9 +52,20 @@ class SampleApp : public ApplicationDelegate, public NativeViewportClient {
   }
 
  private:
+  void OnCreatedNativeViewport(uint64_t native_viewport_id) {
+    CommandBufferPtr cb;
+    // TODO(jamesr): Output to a surface instead.
+    gpu_service_->CreateOnscreenGLES2Context(
+        native_viewport_id, Size::From(gfx::Size(800, 600)), Get(&cb));
+    host_.reset(new CompositorHost(cb.PassMessagePipe()));
+  }
+
+
   NativeViewportPtr viewport_;
   GpuPtr gpu_service_;
   scoped_ptr<CompositorHost> host_;
+  base::WeakPtrFactory<SampleApp> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(SampleApp);
 };
 

mojo/examples/pepper_container_app/pepper_container_app.cc

@@ -5,6 +5,7 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "build/build_config.h"
 #include "mojo/application/application_runner_chromium.h"
@@ -29,9 +30,10 @@ class PepperContainerApp: public ApplicationDelegate,
                           public NativeViewportClient,
                           public MojoPpapiGlobals::Delegate {
  public:
-  explicit PepperContainerApp()
+  PepperContainerApp()
       : ppapi_globals_(this),
-        plugin_module_(new PluginModule) {}
+        plugin_module_(new PluginModule),
+        weak_factory_(this) {}
 
   virtual ~PepperContainerApp() {}
 
@@ -45,20 +47,13 @@ class PepperContainerApp: public ApplicationDelegate,
     SizePtr size(Size::New());
     size->width = 800;
     size->height = 600;
-    viewport_->Create(size.Pass());
+    viewport_->Create(size.Pass(),
+                      base::Bind(&PepperContainerApp::OnCreatedNativeViewport,
+                                 weak_factory_.GetWeakPtr()));
     viewport_->Show();
   }
 
   // NativeViewportClient implementation.
-  virtual void OnCreated(uint64_t native_viewport_id) OVERRIDE {
-    native_viewport_id_ = native_viewport_id;
-    ppapi::ProxyAutoLock lock;
-
-    plugin_instance_ = plugin_module_->CreateInstance().Pass();
-    if (!plugin_instance_->DidCreate())
-      plugin_instance_.reset();
-  }
-
   virtual void OnDestroyed() OVERRIDE {
     ppapi::ProxyAutoLock lock;
 
@@ -70,11 +65,13 @@ class PepperContainerApp: public ApplicationDelegate,
     base::MessageLoop::current()->Quit();
   }
 
-  virtual void OnBoundsChanged(SizePtr bounds) OVERRIDE {
+  virtual void OnSizeChanged(SizePtr size) OVERRIDE {
     ppapi::ProxyAutoLock lock;
 
-    if (plugin_instance_)
-      plugin_instance_->DidChangeView(bounds.To<PP_Rect>());
+    if (plugin_instance_) {
+      PP_Rect pp_rect = {{0, 0}, {size->width, size->height}};
+      plugin_instance_->DidChangeView(pp_rect);
+    }
   }
 
   virtual void OnEvent(EventPtr event,
@@ -100,6 +97,15 @@ class PepperContainerApp: public ApplicationDelegate,
   }
 
  private:
+  void OnCreatedNativeViewport(uint64_t native_viewport_id) {
+    native_viewport_id_ = native_viewport_id;
+    ppapi::ProxyAutoLock lock;
+
+    plugin_instance_ = plugin_module_->CreateInstance().Pass();
+    if (!plugin_instance_->DidCreate())
+      plugin_instance_.reset();
+  }
+
   MojoPpapiGlobals ppapi_globals_;
 
   uint64_t native_viewport_id_;
@@ -108,6 +114,8 @@ class PepperContainerApp: public ApplicationDelegate,
   scoped_refptr<PluginModule> plugin_module_;
   scoped_ptr<PluginInstance> plugin_instance_;
 
+  base::WeakPtrFactory<PepperContainerApp> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(PepperContainerApp);
 };
 

mojo/examples/sample_app/BUILD.gn

@@ -14,6 +14,7 @@ shared_library("sample_app") {
 
   deps = [
     ":spinning_cube",
+    "//base",
     "//gpu/command_buffer/client:gles2_interface",
     "//mojo/public/c/system:for_shared_library",
     "//mojo/public/cpp/application:standalone",

mojo/examples/sample_app/sample_app.cc

@@ -5,6 +5,8 @@
 #include <stdio.h>
 #include <string>
 
+#include "base/bind.h"
+#include "base/memory/weak_ptr.h"
 #include "mojo/examples/sample_app/gles2_client_impl.h"
 #include "mojo/public/c/system/main.h"
 #include "mojo/public/cpp/application/application_connection.h"
@@ -22,7 +24,7 @@ namespace examples {
 class SampleApp : public mojo::ApplicationDelegate,
                   public mojo::NativeViewportClient {
  public:
-  SampleApp() {}
+  SampleApp() : weak_factory_(this) {}
 
   virtual ~SampleApp() {
     // TODO(darin): Fix shutdown so we don't need to leak this.
@@ -39,27 +41,18 @@ class SampleApp : public mojo::ApplicationDelegate,
     mojo::SizePtr size(mojo::Size::New());
     size->width = 800;
     size->height = 600;
-    viewport_->Create(size.Pass());
+    viewport_->Create(size.Pass(),
+                      base::Bind(&SampleApp::OnCreatedNativeViewport,
+                                 weak_factory_.GetWeakPtr()));
     viewport_->Show();
   }
 
-  virtual void OnCreated(uint64_t native_viewport_id) MOJO_OVERRIDE {
-    mojo::SizePtr size = mojo::Size::New();
-    size->width = 800;
-    size->height = 600;
-    mojo::CommandBufferPtr command_buffer;
-    // TODO(jamesr): Output to a surface instead.
-    gpu_service_->CreateOnscreenGLES2Context(
-        native_viewport_id, size.Pass(), Get(&command_buffer));
-    gles2_client_.reset(new GLES2ClientImpl(command_buffer.Pass()));
-  }
-
   virtual void OnDestroyed() MOJO_OVERRIDE { mojo::RunLoop::current()->Quit(); }
 
-  virtual void OnBoundsChanged(mojo::SizePtr bounds) MOJO_OVERRIDE {
-    assert(bounds);
+  virtual void OnSizeChanged(mojo::SizePtr size) MOJO_OVERRIDE {
+    assert(size);
     if (gles2_client_)
-      gles2_client_->SetSize(*bounds);
+      gles2_client_->SetSize(*size);
   }
 
   virtual void OnEvent(mojo::EventPtr event,
@@ -71,9 +64,21 @@ class SampleApp : public mojo::ApplicationDelegate,
   }
 
  private:
+  void OnCreatedNativeViewport(uint64_t native_viewport_id) {
+    mojo::SizePtr size = mojo::Size::New();
+    size->width = 800;
+    size->height = 600;
+    mojo::CommandBufferPtr command_buffer;
+    // TODO(jamesr): Output to a surface instead.
+    gpu_service_->CreateOnscreenGLES2Context(
+        native_viewport_id, size.Pass(), Get(&command_buffer));
+    gles2_client_.reset(new GLES2ClientImpl(command_buffer.Pass()));
+  }
+
   scoped_ptr<GLES2ClientImpl> gles2_client_;
   mojo::NativeViewportPtr viewport_;
   mojo::GpuPtr gpu_service_;
+  base::WeakPtrFactory<SampleApp> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(SampleApp);
 };

mojo/examples/surfaces_app/surfaces_app.cc

@@ -4,6 +4,7 @@
 
 #include "base/bind.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "cc/surfaces/surface_id_allocator.h"
 #include "mojo/application/application_runner_chromium.h"
@@ -29,7 +30,7 @@ class SurfacesApp : public ApplicationDelegate,
                     public SurfaceClient,
                     public NativeViewportClient {
  public:
-  SurfacesApp() {}
+  SurfacesApp() : weak_factory_(this) {}
   virtual ~SurfacesApp() {}
 
   // ApplicationDelegate implementation
@@ -46,7 +47,9 @@ class SurfacesApp : public ApplicationDelegate,
 
     size_ = gfx::Size(800, 600);
 
-    viewport_->Create(Size::From(size_));
+    viewport_->Create(Size::From(size_),
+                      base::Bind(&SurfacesApp::OnCreatedNativeViewport,
+                                 weak_factory_.GetWeakPtr()));
     viewport_->Show();
 
     child_size_ = gfx::Size(size_.width() / 3, size_.height() / 2);
@@ -103,8 +106,7 @@ class SurfacesApp : public ApplicationDelegate,
     DCHECK(!resources.size());
   }
   // NativeViewportClient implementation.
-  virtual void OnCreated(uint64_t native_viewport_id) OVERRIDE {}
-  virtual void OnBoundsChanged(mojo::SizePtr bounds) OVERRIDE {}
+  virtual void OnSizeChanged(mojo::SizePtr size) OVERRIDE {}
   virtual void OnDestroyed() OVERRIDE {}
   virtual void OnEvent(mojo::EventPtr event,
                        const mojo::Callback<void()>& callback) OVERRIDE {
@@ -112,6 +114,8 @@ class SurfacesApp : public ApplicationDelegate,
   }
 
  private:
+  void OnCreatedNativeViewport(uint64_t native_viewport_id) {}
+
   SurfacesServicePtr surfaces_service_;
   SurfacePtr surface_;
   cc::SurfaceId onscreen_id_;
@@ -126,6 +130,8 @@ class SurfacesApp : public ApplicationDelegate,
 
   NativeViewportPtr viewport_;
 
+  base::WeakPtrFactory<SurfacesApp> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(SurfacesApp);
 };
 

mojo/mojo_examples.gypi

@@ -68,6 +68,7 @@
       'target_name': 'mojo_sample_app',
       'type': 'loadable_module',
       'dependencies': [
+        '../base/base.gyp:base',
         'mojo_base.gyp:mojo_application_standalone',
         'mojo_base.gyp:mojo_cpp_bindings',
         'mojo_base.gyp:mojo_utility',

mojo/services/native_viewport/native_viewport_impl.cc

@@ -45,14 +45,16 @@ NativeViewportImpl::~NativeViewportImpl() {
   platform_viewport_.reset();
 }
 
-void NativeViewportImpl::Create(SizePtr bounds) {
+void NativeViewportImpl::Create(SizePtr size,
+                                const Callback<void(uint64_t)>& callback) {
+  create_callback_ = callback;
   if (is_headless_)
     platform_viewport_ = PlatformViewportHeadless::Create(this);
   else
     platform_viewport_ = PlatformViewport::Create(this);
-  gfx::Rect rect = gfx::Rect(bounds.To<gfx::Size>());
-  platform_viewport_->Init(rect);
-  OnBoundsChanged(rect);
+  const gfx::Rect bounds(gfx::Rect(size.To<gfx::Size>()));
+  platform_viewport_->Init(bounds);
+  OnBoundsChanged(bounds);
 }
 
 void NativeViewportImpl::Show() {
@@ -68,8 +70,8 @@ void NativeViewportImpl::Close() {
   platform_viewport_->Close();
 }
 
-void NativeViewportImpl::SetBounds(SizePtr bounds) {
-  platform_viewport_->SetBounds(gfx::Rect(bounds.To<gfx::Size>()));
+void NativeViewportImpl::SetSize(SizePtr size) {
+  platform_viewport_->SetBounds(gfx::Rect(size.To<gfx::Size>()));
 }
 
 void NativeViewportImpl::SubmittedFrame(SurfaceIdPtr child_surface_id) {
@@ -81,7 +83,7 @@ void NativeViewportImpl::SubmittedFrame(SurfaceIdPtr child_surface_id) {
     viewport_surface_.reset(
         new ViewportSurface(surfaces_service_.get(),
                             gpu_service_.get(),
-                            bounds_.size(),
+                            size_,
                             child_surface_id.To<cc::SurfaceId>()));
     if (widget_id_)
       viewport_surface_->SetWidgetId(widget_id_);
@@ -92,17 +94,17 @@ void NativeViewportImpl::SubmittedFrame(SurfaceIdPtr child_surface_id) {
 }
 
 void NativeViewportImpl::OnBoundsChanged(const gfx::Rect& bounds) {
-  bounds_ = bounds;
-  client()->OnBoundsChanged(Size::From(bounds.size()));
+  size_ = bounds.size();
+  client()->OnSizeChanged(Size::From(size_));
   if (viewport_surface_)
-    viewport_surface_->SetSize(bounds.size());
+    viewport_surface_->SetSize(size_);
 }
 
 void NativeViewportImpl::OnAcceleratedWidgetAvailable(
     gfx::AcceleratedWidget widget) {
   widget_id_ = static_cast<uint64_t>(bit_cast<uintptr_t>(widget));
   // TODO(jamesr): Remove once everything is converted to surfaces.
-  client()->OnCreated(widget_id_);
+  create_callback_.Run(widget_id_);
   if (viewport_surface_)
     viewport_surface_->SetWidgetId(widget_id_);
 }

mojo/services/native_viewport/native_viewport_impl.h

@@ -28,11 +28,12 @@ class NativeViewportImpl : public InterfaceImpl<NativeViewport>,
   virtual ~NativeViewportImpl();
 
   // InterfaceImpl<NativeViewport> implementation.
-  virtual void Create(SizePtr bounds) OVERRIDE;
+  virtual void Create(SizePtr size,
+                      const Callback<void(uint64_t)>& callback) OVERRIDE;
   virtual void Show() OVERRIDE;
   virtual void Hide() OVERRIDE;
   virtual void Close() OVERRIDE;
-  virtual void SetBounds(SizePtr bounds) OVERRIDE;
+  virtual void SetSize(SizePtr size) OVERRIDE;
   virtual void SubmittedFrame(SurfaceIdPtr surface_id) OVERRIDE;
 
   // PlatformViewport::Delegate implementation.
@@ -49,11 +50,12 @@ class NativeViewportImpl : public InterfaceImpl<NativeViewport>,
   scoped_ptr<PlatformViewport> platform_viewport_;
   scoped_ptr<ViewportSurface> viewport_surface_;
   uint64_t widget_id_;
-  gfx::Rect bounds_;
+  gfx::Size size_;
   GpuPtr gpu_service_;
   SurfacesServicePtr surfaces_service_;
   cc::SurfaceId child_surface_id_;
   bool waiting_for_event_ack_;
+  Callback<void(uint64_t)> create_callback_;
   base::WeakPtrFactory<NativeViewportImpl> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(NativeViewportImpl);

mojo/services/public/interfaces/native_viewport/native_viewport.mojom

@@ -11,17 +11,19 @@ module mojo {
 
 [Client=NativeViewportClient]
 interface NativeViewport {
-  Create(Size size);
+  // TODO(sky): having a create function is awkward. Should there be a factory
+  // to create the NativeViewport that takes the size?
+  // TODO(sky): callback should take size too.
+  Create(Size size) => (uint64 native_viewport_id);
   Show();
   Hide();
   Close();
-  SetBounds(Size size);
+  SetSize(Size size);
   SubmittedFrame(SurfaceId surface_id);
 };
 
 interface NativeViewportClient {
-  OnCreated(uint64 native_viewport_id);
-  OnBoundsChanged(Size size);
+  OnSizeChanged(Size size);
   OnDestroyed();
   OnEvent(Event event) => ();
 };

mojo/services/view_manager/display_manager.cc

@@ -74,13 +74,16 @@ DisplayManager::DisplayManager(
     const Callback<void()>& native_viewport_closed_callback)
     : connection_manager_(connection_manager),
       in_setup_(false),
-      bounds_(800, 600),
+      size_(800, 600),
       draw_timer_(false, false),
       weak_factory_(this) {
   app_connection->ConnectToService("mojo:mojo_native_viewport_service",
                                    &native_viewport_);
   native_viewport_.set_client(this);
-  native_viewport_->Create(Size::From(bounds_));
+  native_viewport_->Create(
+      Size::From(size_),
+      base::Bind(&DisplayManager::OnCreatedNativeViewport,
+                 weak_factory_.GetWeakPtr()));
   native_viewport_->Show();
   app_connection->ConnectToService("mojo:mojo_surfaces_service",
                                    &surfaces_service_);
@@ -107,6 +110,9 @@ void DisplayManager::SchedulePaint(const ServerView* view,
   }
 }
 
+void DisplayManager::OnCreatedNativeViewport(uint64_t native_viewport_id) {
+}
+
 void DisplayManager::OnSurfaceConnectionCreated(SurfacePtr surface,
                                                 uint32_t id_namespace) {
   surface_ = surface.Pass();
@@ -120,10 +126,10 @@ void DisplayManager::Draw() {
     return;
   if (surface_id_.is_null()) {
     surface_id_ = surface_id_allocator_->GenerateId();
-    surface_->CreateSurface(SurfaceId::From(surface_id_), Size::From(bounds_));
+    surface_->CreateSurface(SurfaceId::From(surface_id_), Size::From(size_));
   }
 
-  PassPtr pass = CreateDefaultPass(1, gfx::Rect(bounds_));
+  PassPtr pass = CreateDefaultPass(1, gfx::Rect(size_));
   pass->damage_rect = Rect::From(dirty_rect_);
 
   DrawViewTree(pass.get(), connection_manager_->root(), gfx::Vector2d());
@@ -138,21 +144,18 @@ void DisplayManager::Draw() {
   dirty_rect_ = gfx::Rect();
 }
 
-void DisplayManager::OnCreated(uint64_t native_viewport_id) {
-}
-
 void DisplayManager::OnDestroyed() {
   native_viewport_closed_callback_.Run();
 }
 
-void DisplayManager::OnBoundsChanged(SizePtr bounds) {
-  bounds_ = bounds.To<gfx::Size>();
-  connection_manager_->root()->SetBounds(gfx::Rect(bounds_));
+void DisplayManager::OnSizeChanged(SizePtr size) {
+  size_ = size.To<gfx::Size>();
+  connection_manager_->root()->SetBounds(gfx::Rect(size_));
   if (surface_id_.is_null())
     return;
   surface_->DestroySurface(SurfaceId::From(surface_id_));
   surface_id_ = cc::SurfaceId();
-  SchedulePaint(connection_manager_->root(), gfx::Rect(bounds_));
+  SchedulePaint(connection_manager_->root(), gfx::Rect(size_));
 }
 
 void DisplayManager::OnEvent(EventPtr event,

mojo/services/view_manager/display_manager.h

@@ -49,13 +49,13 @@ class MOJO_VIEW_MANAGER_EXPORT DisplayManager
   bool in_setup() const { return in_setup_; }
 
  private:
+  void OnCreatedNativeViewport(uint64_t native_viewport_id);
   void OnSurfaceConnectionCreated(SurfacePtr surface, uint32_t id_namespace);
   void Draw();
 
   // NativeViewportClient implementation.
-  virtual void OnCreated(uint64_t native_viewport_id) OVERRIDE;
   virtual void OnDestroyed() OVERRIDE;
-  virtual void OnBoundsChanged(SizePtr bounds) OVERRIDE;
+  virtual void OnSizeChanged(SizePtr size) OVERRIDE;
   virtual void OnEvent(EventPtr event,
                        const mojo::Callback<void()>& callback) OVERRIDE;
 
@@ -67,7 +67,7 @@ class MOJO_VIEW_MANAGER_EXPORT DisplayManager
   // Returns true if adding the root view's window to |window_tree_host_|.
   bool in_setup_;
 
-  gfx::Size bounds_;
+  gfx::Size size_;
   gfx::Rect dirty_rect_;
   base::Timer draw_timer_;