Make sure that Content Handled application can be connected multiple times.

examples/png_viewer/png_viewer.cc

Index: examples/png_viewer/png_viewer.cc
diff --git a/examples/png_viewer/png_viewer.cc b/examples/png_viewer/png_viewer.cc
index 3f17dd2f4351290418ec1d750ea7c25554295f9c..df55a0c72c381a2a7a6c9f1be7fe6c32ab5df6d8 100644
--- a/examples/png_viewer/png_viewer.cc
+++ b/examples/png_viewer/png_viewer.cc
@@ -31,6 +31,25 @@
 namespace mojo {
 namespace examples {
 
+namespace {
+
+class EmbedderData {
+ public:
+  EmbedderData(Shell* shell, View* root) : bitmap_uploader_(root) {
+    bitmap_uploader_.Init(shell);
+    bitmap_uploader_.SetColor(SK_ColorGRAY);
+  }
+
+  BitmapUploader& bitmap_uploader() { return bitmap_uploader_; }
+
+ private:
+  BitmapUploader bitmap_uploader_;
+
+  DISALLOW_COPY_AND_ASSIGN(EmbedderData);
+};
+
+}  // namespace
+
 // TODO(aa): Hook up ZoomableMedia interface again.
 class PNGView : public ApplicationDelegate,
                 public ViewManagerDelegate,
@@ -40,14 +59,15 @@ class PNGView : public ApplicationDelegate,
       : width_(0),
         height_(0),
         app_(nullptr),
-        root_(nullptr),
         zoom_percentage_(kDefaultZoomPercentage) {
     DecodePNG(response.Pass());
   }
 
   virtual ~PNGView() {
-    if (root_)
-      root_->RemoveObserver(this);
+    for (auto& roots : embedder_for_roots_) {
+      roots.first->RemoveObserver(this);
+      delete roots.second;
+    }
   }
 
  private:
@@ -76,14 +96,14 @@ class PNGView : public ApplicationDelegate,
                        ServiceProviderImpl* exported_services,
                        scoped_ptr<ServiceProvider> imported_services) override {
     // TODO(qsr): The same view should be embeddable on multiple views.
-    DCHECK(!root_);
-    root_ = root;
-    root_->AddObserver(this);
-    bitmap_uploader_.reset(new BitmapUploader(root_));
-    bitmap_uploader_->Init(app_->shell());
-    bitmap_uploader_->SetColor(SK_ColorGRAY);
-    if (bitmap_.get())
-      DrawBitmap();
+    DCHECK(embedder_for_roots_.find(root) == embedder_for_roots_.end());
+    root->AddObserver(this);
+    EmbedderData* embedder_data = new EmbedderData(app_->shell(), root);
+    embedder_for_roots_[root] = embedder_data;
+    embedder_data->bitmap_uploader().SetBitmap(
+        width_, height_,
+        make_scoped_ptr(new std::vector<unsigned char>(*bitmap_)),
+        BitmapUploader::BGRA);
   }
 
   virtual void OnViewManagerDisconnected(ViewManager* view_manager) override {
@@ -94,45 +114,35 @@ class PNGView : public ApplicationDelegate,
   virtual void OnViewBoundsChanged(View* view,
                                    const Rect& old_bounds,
                                    const Rect& new_bounds) override {
-    DCHECK_EQ(view, root_);
-    DrawBitmap();
+    DCHECK(embedder_for_roots_.find(view) != embedder_for_roots_.end());
   }
 
   virtual void OnViewDestroyed(View* view) override {
-    DCHECK_EQ(view, root_);
-    // TODO(qsr): It should not be necessary to cleanup the uploader, but it
-    // crashes if the GL context goes away.
-    bitmap_uploader_.reset();
-    ApplicationImpl::Terminate();
-  }
-
-  void DrawBitmap() {
-    if (!root_)
-      return;
-
-    bitmap_uploader_->SetBitmap(
-        width_, height_, bitmap_.Pass(), BitmapUploader::BGRA);
+    const auto& it = embedder_for_roots_.find(view);
+    DCHECK(it != embedder_for_roots_.end());
+    delete it->second;
+    embedder_for_roots_.erase(it);
+    if (embedder_for_roots_.size() == 0)
+      ApplicationImpl::Terminate();

[Aaron Boodman, 2014/11/18 23:46:15]

Out of scope for this CL, but I don't think this is the right time to shut down
the app.

[qsr, 2014/11/19 13:42:37]

 Can you elaborate? I can always change it in a next CL, but I'm not sure to
understand why, do you prefer to do this in OnViewManagerDisconnected? Never?

[Aaron Boodman, 2014/11/19 15:58:15]

This depends on the order you receive OnEmbed() and OnViewDestroyed(). That
seems pretty brittle to rely upon.

I think if you wanted something like this you could listen to
OnViewManagerDisconnected(). Because once that happens you know that you cannot
receive any more messages from the view manager.

But in general, this doesn't seem like the right way to shut down mojo apps. How
do you know that some other service is not doing something? Or won't begin doing
something shortly?

[qsr, 2014/11/19 16:37:21]

 Hum... I really hope I can count on receiving OnEmbed before receiving
OnViewDestroyed for the same view, otherwise, this cannot work at all... And if
this work, then I do not understand why this is brittle. I'm clearly missing
something.
 Hum... Then I probably also need to keep track of connected app by keeping
track of ServiceProvider. Again, if you do not mind, I'll let that for another
CL?

[Aaron Boodman, 2014/11/20 00:59:01]

No what I mean is that, during navigation for example, you might receive:

Embed(v1)
Embed(v2)
Destroy(v1)
Destroy(v2)

Or you might receive:

Embed(v1)
Destroy(v1)
Embed(v2)
Destroy(v2)

Both are reasonable orders for the VM to dispatch these messages in, depending
on implementation details.

But in the second example, you will shut down the app before Embed(v2) happens.
I think it would be better if apps did not depend on the order of these
events...
What I meant to convey is that shutdown is going to be a much larger problem
than this one CL or this one app. We should probably not write any shut down
code until we have a unified plan for how it's going to work for all apps. It's
just more code getting in the way.

Anyway this was already here before your CL, so you can just leave it.

   }
 
   void ZoomIn() {
+    // TODO(qsr,aa) Zoom should be per embedder view.
     if (zoom_percentage_ >= kMaxZoomPercentage)
       return;
     zoom_percentage_ += kZoomStep;
-    DrawBitmap();
   }
 
   void ZoomOut() {
     if (zoom_percentage_ <= kMinZoomPercentage)
       return;
     zoom_percentage_ -= kZoomStep;
-    DrawBitmap();
   }
 
   void ZoomToActualSize() {
     if (zoom_percentage_ == kDefaultZoomPercentage)
       return;
     zoom_percentage_ = kDefaultZoomPercentage;
-    DrawBitmap();
   }
 
   void DecodePNG(URLResponsePtr response) {
@@ -142,11 +152,10 @@ class PNGView : public ApplicationDelegate,
     uint32_t bytes_remaining = content_length;
     uint32_t num_bytes = bytes_remaining;
     while (bytes_remaining > 0) {
-      MojoResult result = ReadDataRaw(
-          response->body.get(), buf, &num_bytes, MOJO_READ_DATA_FLAG_NONE);
+      MojoResult result = ReadDataRaw(response->body.get(), buf, &num_bytes,
+                                      MOJO_READ_DATA_FLAG_NONE);
       if (result == MOJO_RESULT_SHOULD_WAIT) {
-        Wait(response->body.get(),
-             MOJO_HANDLE_SIGNAL_READABLE,
+        Wait(response->body.get(), MOJO_HANDLE_SIGNAL_READABLE,
              MOJO_DEADLINE_INDEFINITE);
       } else if (result == MOJO_RESULT_OK) {
         buf += num_bytes;
@@ -158,11 +167,8 @@ class PNGView : public ApplicationDelegate,
 
     bitmap_.reset(new std::vector<unsigned char>);
     gfx::PNGCodec::Decode(static_cast<const unsigned char*>(data.get()),
-                          content_length,
-                          gfx::PNGCodec::FORMAT_BGRA,
-                          bitmap_.get(),
-                          &width_,
-                          &height_);
+                          content_length, gfx::PNGCodec::FORMAT_BGRA,
+                          bitmap_.get(), &width_, &height_);
   }
 
   int GetContentLength(const Array<String>& headers) {
@@ -184,10 +190,9 @@ class PNGView : public ApplicationDelegate,
   int height_;
   scoped_ptr<std::vector<unsigned char>> bitmap_;
   ApplicationImpl* app_;
-  View* root_;
+  std::map<View*, EmbedderData*> embedder_for_roots_;
   scoped_ptr<ViewManagerClientFactory> view_manager_client_factory_;
   uint16_t zoom_percentage_;
-  scoped_ptr<BitmapUploader> bitmap_uploader_;
 
   DISALLOW_COPY_AND_ASSIGN(PNGView);
 };