Fix html viewer.

mojo/services/html_viewer/html_document_view.cc

 // Copyright 2014 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.
 
 #include "mojo/services/html_viewer/html_document_view.h"
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/single_thread_task_runner.h"
@@ skipping 68 matching lines @@
     return true;
 
   // Otherwise we don't know if we're the right app to handle this request. Ask
   // host to do the navigation for us.
   return false;
 }
 
 }  // namespace
 
 HTMLDocumentView::HTMLDocumentView(
+    const base::Closure& destruction_callback,
+    mojo::ServiceProviderPtr provider,
     URLResponsePtr response,
-    ShellPtr shell,
+    Shell* shell,
     scoped_refptr<base::MessageLoopProxy> compositor_thread,
     WebMediaPlayerFactory* web_media_player_factory)
-    : response_(response.Pass()),
-      shell_(shell.Pass()),
+    : destruction_callback_(destruction_callback),
+      response_(response.Pass()),
+      shell_(shell),

[Aaron Boodman, 2014/12/03 16:42:11]

This raw ptr is another way we can crash if the application goes away before us.

[qsr, 2014/12/03 16:56:53]

 This raw ptr is owned by the application, and the application cannot delete
itself before this class is deleted.

[Aaron Boodman, 2014/12/03 17:32:37]

Right.

       web_view_(NULL),
       root_(NULL),
-      view_manager_client_factory_(shell_.get(), this),
+      view_manager_client_factory_(shell_, this),
       compositor_thread_(compositor_thread),
       web_media_player_factory_(web_media_player_factory) {
-  shell_.set_client(this);
+  exported_services_.AddService(this);
+  exported_services_.AddService(&view_manager_client_factory_);
+  WeakBindToPipe(&exported_services_, provider.PassMessagePipe());

[Aaron Boodman, 2014/12/03 16:42:11]

What deletes this class?

[qsr, 2014/12/03 16:56:53]

Which class? exported_services_ is a member of this HTMLDocumentView, so it is
deleted at the same time as HTMLDocumentView

But I miss a delete this in OnViewDestroyed

[Aaron Boodman, 2014/12/03 17:32:37]

I meant HTMLDocumentView.

I don't think OnViewDestroyed is the right place to delete this. It is possible
to create an HTMLDocumentView, but never have any associated view. In that case
OnViewDestroyed won't ever be called.

[qsr, 2014/12/04 10:11:17]

 Hum, what would be the right signal? This is not the shell closing the
application, you can imagine this with some more connection to this. This is not
the other side closing its ServiceProvider, because it can do this after getting
a reference to the embedder service
 

 }
 
 HTMLDocumentView::~HTMLDocumentView() {
   STLDeleteElements(&ax_provider_impls_);
 
   if (web_view_)
     web_view_->close();
   if (root_)
     root_->RemoveObserver(this);
-}
-
-void HTMLDocumentView::AcceptConnection(const String& requestor_url,
-                                        ServiceProviderPtr provider) {
-  exported_services_.AddService(this);
-  exported_services_.AddService(&view_manager_client_factory_);
-  WeakBindToPipe(&exported_services_, provider.PassMessagePipe());
-  Load(response_.Pass());
-}
-
-void HTMLDocumentView::Initialize(Array<String> args) {
+  destruction_callback_.Run();
 }
 
 void HTMLDocumentView::OnEmbed(
     ViewManager* view_manager,
     View* root,
     ServiceProviderImpl* embedee_service_provider_impl,
     scoped_ptr<ServiceProvider> embedder_service_provider) {
   root_ = root;
   embedder_service_provider_ = embedder_service_provider.Pass();
   navigator_host_.set_service_provider(embedder_service_provider_.get());
 
+  Load(response_.Pass());
+
   blink::WebSize root_size(root_->bounds().width, root_->bounds().height);
   web_view_->resize(root_size);
   web_layer_tree_view_impl_->setViewportSize(root_size);
   web_layer_tree_view_impl_->set_view(root_);
   root_->AddObserver(this);
 }
 
 void HTMLDocumentView::Create(ApplicationConnection* connection,
                               InterfaceRequest<AxProvider> request) {
   if (!web_view_)
@@ skipping 47 matching lines @@
 }
 
 blink::WebLayerTreeView* HTMLDocumentView::layerTreeView() {
   return web_layer_tree_view_impl_.get();
 }
 
 blink::WebMediaPlayer* HTMLDocumentView::createMediaPlayer(
     blink::WebLocalFrame* frame,
     const blink::WebURL& url,
     blink::WebMediaPlayerClient* client) {
-  return web_media_player_factory_->CreateMediaPlayer(
-      frame, url, client, shell_.get());
+  return web_media_player_factory_->CreateMediaPlayer(frame, url, client,
+                                                      shell_);
 }
 
 blink::WebMediaPlayer* HTMLDocumentView::createMediaPlayer(
     blink::WebLocalFrame* frame,
     const blink::WebURL& url,
     blink::WebMediaPlayerClient* client,
     blink::WebContentDecryptionModule* initial_cdm) {
   return createMediaPlayer(frame, url, client);
 }
 
@@ skipping 61 matching lines @@
 }
 
 void HTMLDocumentView::OnViewInputEvent(View* view, const EventPtr& event) {
   scoped_ptr<blink::WebInputEvent> web_event =
       event.To<scoped_ptr<blink::WebInputEvent> >();
   if (web_event)
     web_view_->handleInputEvent(*web_event);
 }
 
 }  // namespace mojo