Fix html viewer.

mojo/services/html_viewer/html_viewer.cc

Index: mojo/services/html_viewer/html_viewer.cc
diff --git a/mojo/services/html_viewer/html_viewer.cc b/mojo/services/html_viewer/html_viewer.cc
index 4db44ad3714ae35c56049a7c11b65d0a054a8b09..265d2e357795e1c17b70e1739f9951328367908f 100644
--- a/mojo/services/html_viewer/html_viewer.cc
+++ b/mojo/services/html_viewer/html_viewer.cc
@@ -13,11 +13,13 @@
 #include "mojo/public/cpp/application/application_connection.h"
 #include "mojo/public/cpp/application/application_delegate.h"
 #include "mojo/public/cpp/application/application_impl.h"
+#include "mojo/public/cpp/application/connect.h"
 #include "mojo/public/cpp/application/interface_factory_impl.h"
 #include "mojo/services/html_viewer/html_document_view.h"
 #include "mojo/services/html_viewer/mojo_blink_platform_impl.h"
 #include "mojo/services/html_viewer/webmediaplayer_factory.h"
 #include "mojo/services/public/interfaces/content_handler/content_handler.mojom.h"
+#include "mojo/services/public/interfaces/network/network_service.mojom.h"
 #include "third_party/WebKit/public/web/WebKit.h"
 
 #if !defined(COMPONENT_BUILD)
@@ -32,12 +34,84 @@ namespace mojo {
 // Switches for html_viewer to be used with "--args-for". For example:
 // --args-for='mojo:html_viewer --enable-mojo-media-renderer'
 
-// Enable mojo::MediaRenderer in media pipeline instead of using the internal
+// Enable MediaRenderer in media pipeline instead of using the internal
 // media::Renderer implementation.
 const char kEnableMojoMediaRenderer[] = "enable-mojo-media-renderer";
 
 class HTMLViewer;
 
+class HTMLViewerApplication : public Application {
+ public:
+  HTMLViewerApplication(ShellPtr shell,
+                        URLResponsePtr response,
+                        scoped_refptr<base::MessageLoopProxy> compositor_thread,
+                        WebMediaPlayerFactory* web_media_player_factory)
+      : url_(response->url),
+        shell_(shell.Pass()),
+        initial_response_(response.Pass()),
+        compositor_thread_(compositor_thread),
+        web_media_player_factory_(web_media_player_factory),
+        view_count_(0) {
+    shell_.set_client(this);
+    ServiceProviderPtr service_provider;
+    shell_->ConnectToApplication("mojo:network_service",
+                                 GetProxy(&service_provider));
+    ConnectToService(service_provider.get(), &network_service_);
+  }
+
+  void Initialize(Array<String> args) override {}
+
+  void AcceptConnection(const String& requestor_url,
+                        ServiceProviderPtr provider) override {
+    ++view_count_;
+    if (initial_response_) {
+      OnResponseReceived(URLLoaderPtr(), provider.Pass(),
+                         initial_response_.Pass());
+    } else {
+      URLLoaderPtr loader;
+      network_service_->CreateURLLoader(GetProxy(&loader));
+      URLRequestPtr request(URLRequest::New());
+      request->url = url_;
+      request->auto_follow_redirects = true;
+
+      // |loader| will be pass to the OnResponseReceived method through a
+      // callback. Because order of evaluation is undefined, a reference to the
+      // raw pointer is needed.
+      URLLoader* raw_loader = loader.get();
+      raw_loader->Start(
+          request.Pass(),
+          base::Bind(&HTMLViewerApplication::OnResponseReceived,
+                     base::Unretained(this), base::Passed(&loader),
+                     base::Passed(&provider)));
+    }
+  }
+
+ private:
+  void OnViewDestroyed() {
+    --view_count_;
+    if (view_count_ == 0) {
+      delete this;
+    }
+  }
+
+  void OnResponseReceived(URLLoaderPtr loader,
+                          ServiceProviderPtr provider,
+                          URLResponsePtr response) {
+    new HTMLDocumentView(base::Bind(&HTMLViewerApplication::OnViewDestroyed,

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

I know this already existed, but naming nit: this thing isn't really a 'view',
as it has nothing to do with an embedding in ViewManager. It's really just an
HTMLDocument. We should rename it.

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

Done.

+                                    base::Unretained(this)),

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

Won't the application crash if HTMLViewerApplication is destroyed before one of
the HTMLDocumentView's it created is?

Perhaps HTMLViewerApplication should either directly own, or maintain raw
pointers to, the HTMLDocumentViews it creates. Then when HTMLViewerApplication
goes away, it can take all the remaining HTMLDocumentView instances with it.

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

 HTMLViewerApplication is self owned. It can only delete itself when there is no
more HTMLDocumentView

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

Ah, true.

+                         provider.Pass(), response.Pass(), shell_.get(),
+                         compositor_thread_, web_media_player_factory_);
+  }
+
+  String url_;
+  ShellPtr shell_;
+  NetworkServicePtr network_service_;
+  URLResponsePtr initial_response_;
+  scoped_refptr<base::MessageLoopProxy> compositor_thread_;
+  WebMediaPlayerFactory* web_media_player_factory_;
+  uint32_t view_count_;
+};
+
 class ContentHandlerImpl : public InterfaceImpl<ContentHandler> {
  public:
   ContentHandlerImpl(scoped_refptr<base::MessageLoopProxy> compositor_thread,
@@ -49,10 +123,8 @@ class ContentHandlerImpl : public InterfaceImpl<ContentHandler> {
  private:
   // Overridden from ContentHandler:
   void StartApplication(ShellPtr shell, URLResponsePtr response) override {
-    new HTMLDocumentView(response.Pass(),
-                         shell.Pass(),
-                         compositor_thread_,
-                         web_media_player_factory_);
+    new HTMLViewerApplication(shell.Pass(), response.Pass(), compositor_thread_,
+                              web_media_player_factory_);
   }
 
   scoped_refptr<base::MessageLoopProxy> compositor_thread_;