Update view_manager and window_manager to make use of content handling.

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 aa8d46bfb299d1bc5330006a5c5aa56880c4da95..dffd5ec25a3023c322df0db81744bd232025a055 100644
--- a/mojo/services/html_viewer/html_viewer.cc
+++ b/mojo/services/html_viewer/html_viewer.cc
@@ -9,109 +9,74 @@
 #include "mojo/public/cpp/application/application_impl.h"
 #include "mojo/public/cpp/application/application_runner_chromium.h"
 #include "mojo/public/cpp/application/interface_factory_impl.h"
+#include "mojo/public/cpp/application/service_provider_impl.h"
 #include "mojo/services/html_viewer/blink_platform_impl.h"
 #include "mojo/services/html_viewer/html_document_view.h"
-#include "mojo/services/public/cpp/view_manager/types.h"
-#include "mojo/services/public/cpp/view_manager/view.h"
-#include "mojo/services/public/cpp/view_manager/view_manager.h"
-#include "mojo/services/public/cpp/view_manager/view_manager_client_factory.h"
-#include "mojo/services/public/cpp/view_manager/view_manager_delegate.h"
-#include "mojo/services/public/interfaces/navigation/navigation.mojom.h"
+#include "mojo/services/public/interfaces/content_handler/content_handler.mojom.h"
 #include "third_party/WebKit/public/web/WebKit.h"
 
 namespace mojo {
 
 class HTMLViewer;
 
-class NavigatorImpl : public InterfaceImpl<Navigator> {
+class ContentHandlerImpl : public InterfaceImpl<ContentHandler> {
  public:
-  explicit NavigatorImpl(HTMLViewer* viewer) : viewer_(viewer) {}
-  virtual ~NavigatorImpl() {}
+  explicit ContentHandlerImpl(ApplicationConnection* application_connection)
+      : application_connection_(application_connection) {
+  }
+  virtual ~ContentHandlerImpl() {}
 
  private:
-  // Overridden from Navigator:
-  virtual void Navigate(
-      uint32_t view_id,
-      NavigationDetailsPtr navigation_details,
-      ResponseDetailsPtr response_details) OVERRIDE;
+  // Overridden from ContentHandler:
+  virtual void OnConnect(
+      const mojo::String& url,
+      URLResponsePtr response,
+      InterfaceRequest<ServiceProvider> service_provider) OVERRIDE {
+    ServiceProviderImpl* exported_services =
+        new ServiceProviderImpl(application_connection_);
+    BindToRequest(exported_services, &service_provider);
+    scoped_ptr<ServiceProvider> remote(
+        exported_services->CreateRemoteServiceProvider());
+    new HTMLDocumentView(response.Pass(), remote.Pass(),
+                         exported_services, application_connection_);
+  }
 
-  HTMLViewer* viewer_;
+  ApplicationConnection* application_connection_;
 
-  DISALLOW_COPY_AND_ASSIGN(NavigatorImpl);
+  DISALLOW_COPY_AND_ASSIGN(ContentHandlerImpl);
 };
 
-class HTMLViewer : public ApplicationDelegate, public ViewManagerDelegate {
+class HTMLViewer : public ApplicationDelegate {
  public:
   HTMLViewer()
-      : application_impl_(NULL),
-        document_view_(NULL),
-        navigator_factory_(this),
-        view_manager_client_factory_(this) {}
-  virtual ~HTMLViewer() {
-    blink::shutdown();
+      : content_handler_factory_(NULL) {
   }
 
-  void Load(ResponseDetailsPtr response_details) {
-    // Need to wait for OnEmbed.
-    response_details_ = response_details.Pass();
-    MaybeLoad();
+  virtual ~HTMLViewer() {
+    blink::shutdown();
   }
 
  private:
   // Overridden from ApplicationDelegate:
   virtual void Initialize(ApplicationImpl* app) OVERRIDE {
-    application_impl_ = app;
     blink_platform_impl_.reset(new BlinkPlatformImpl(app));
     blink::initialize(blink_platform_impl_.get());
   }
 
   virtual bool ConfigureIncomingConnection(ApplicationConnection* connection)
       OVERRIDE {
-    connection->AddService(&navigator_factory_);
-    connection->AddService(&view_manager_client_factory_);
+    content_handler_factory_.set_context(connection);

[darin (slow to review), 2014/08/29 05:59:42]

I'm a bit confused by this code that keeps changing the context associated with
the content_handler_factory_. If ConfigureIncomingConnection is called more than
once but the ContentHandlerImpl is instantiated in reverse order, then wouldn't
we have a bug?

Note: This is probably a good reason why ApplicationConnection should not have
the Shell functionality of ConnectToApplication. That functionality does not
vary from ApplicationConnection instance to ApplicationConnection instance.
Rather, it is just a function of the Shell. It is the Shell that you should make
be a context parameter of the content_handler_factory_. Then it would be
something set once rather than reset like this. The |set_context| method
probably shouldn't exist as it creates dangerous situations.

[Aaron Boodman, 2014/08/29 07:45:11]

Good point.
I agree.

The main reason I was trying to get ApplicationConnection* into
ContentHandlerImpl was actually not so much for HTMLDocumentView, but so that I
could pass it into ServiceProviderImpl. This is because SPI passes it to
ServiceConnector, who passes it to InterfaceFactory::Create(), where it finally
makes it way to ViewManagerClient, who uses it to get to WindowManager.

I was trying to figure out a temporary way to meet that expectation, then go
back and rethink how this should actually work in a later change.

===

Stepping back now though...

I really like your idea of just using Shell to make outbound "global"
connections, rather than ApplicationConnection. There are so many ways to make
outbound connections, it seems like reducing that would be good in general.

What if I remove ApplicationConnection* from InterfaceFactory::Create()
entirely. There are only two implementations that use it
(ViewManagerClientFactory and NetworkApplicationLoader), but those can be
reworked so that the requirements are passed into the constructors instead.

I was also wondering if maybe ApplicationConnection shouldn't just go away. It
seems to overlap in functionality with ServiceProviderImpl... or am I
misunderstanding?

+    connection->AddService(&content_handler_factory_);
     return true;
   }
 
-  // Overridden from ViewManagerDelegate:
-  virtual void OnEmbed(ViewManager* view_manager,
-                       View* root,
-                       ServiceProviderImpl* exported_services,
-                       scoped_ptr<ServiceProvider> imported_services) OVERRIDE {
-    document_view_ = new HTMLDocumentView(
-        application_impl_->ConnectToApplication("mojo://mojo_window_manager/")->
-            GetServiceProvider(),
-        view_manager);
-    document_view_->AttachToView(root);
-    MaybeLoad();
-  }
-  virtual void OnViewManagerDisconnected(ViewManager* view_manager) OVERRIDE {
-    base::MessageLoop::current()->Quit();
-  }
-
-  void MaybeLoad() {
-    if (document_view_ && response_details_ && response_details_->response)
-      document_view_->Load(response_details_->response.Pass());
-  }
-
   scoped_ptr<BlinkPlatformImpl> blink_platform_impl_;
-  ApplicationImpl* application_impl_;
-
-  // TODO(darin): Figure out proper ownership of this instance.
-  HTMLDocumentView* document_view_;
-  ResponseDetailsPtr response_details_;
-  InterfaceFactoryImplWithContext<NavigatorImpl, HTMLViewer> navigator_factory_;
-  ViewManagerClientFactory view_manager_client_factory_;
+  InterfaceFactoryImplWithContext<ContentHandlerImpl, ApplicationConnection>
+      content_handler_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(HTMLViewer);
 };
 
-void NavigatorImpl::Navigate(
-    uint32_t view_id,
-    NavigationDetailsPtr navigation_details,
-    ResponseDetailsPtr response_details) {
-  viewer_->Load(response_details.Pass());
-}
-
 }  // namespace mojo
 
 MojoResult MojoMain(MojoHandle shell_handle) {