Migrate Linux Blimp client to use BlimpClientContext

blimp/client/app/linux/blimp_main.cc

Index: blimp/client/app/linux/blimp_main.cc
diff --git a/blimp/client/app/linux/blimp_main.cc b/blimp/client/app/linux/blimp_main.cc
index 37c7c957fab2e1b06fb4030ec406acce22ff6490..079f017dc758c16ac66f650f2a8be1b3ae8a4069 100644
--- a/blimp/client/app/linux/blimp_main.cc
+++ b/blimp/client/app/linux/blimp_main.cc
@@ -1,4 +1,4 @@
-// Copyright 2015 The Chromium Authors. All rights reserved.
+// Copyright 2016 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.
 
@@ -10,16 +10,22 @@
 #include "base/threading/thread.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "blimp/client/app/blimp_startup.h"
-#include "blimp/client/app/linux/blimp_client_session_linux.h"
-#include "blimp/client/core/contents/navigation_feature.h"
-#include "blimp/client/core/contents/tab_control_feature.h"
-#include "blimp/client/core/session/assignment_source.h"
+#include "blimp/client/app/linux/blimp_client_context_delegate_linux.h"
+#include "blimp/client/app/linux/blimp_display_manager.h"
+#include "blimp/client/app/linux/blimp_display_manager_delegate_main.h"
+#include "blimp/client/public/blimp_client_context.h"
+#include "blimp/client/public/contents/blimp_contents_view.h"
+#include "blimp/client/public/contents/blimp_navigation_controller.h"
+#include "blimp/client/support/compositor/blimp_embedder_compositor.h"
+#include "blimp/client/support/compositor/compositor_dependencies_impl.h"
+#include "blimp/common/get_client_auth_token.h"
+#include "ui/events/platform/platform_event_source.h"
 #include "ui/gfx/x/x11_connection.h"
 
 namespace {
-const char kDummyLoginToken[] = "";
 const char kDefaultUrl[] = "https://www.google.com";
-const int kDummyTabId = 0;
+const int kWindowWidth = 800;

[Kevin M, 2016/09/27 21:59:41]

constexpr

[steimel, 2016/09/27 23:23:19]

Done.

+const int kWindowHeight = 600;
 }
 
 int main(int argc, const char**argv) {
@@ -31,15 +37,45 @@ int main(int argc, const char**argv) {
   blimp::client::InitializeLogging();
   blimp::client::InitializeMainMessageLoop();
 
-  blimp::client::BlimpClientSessionLinux session;
-  session.GetTabControlFeature()->CreateTab(kDummyTabId);
-  session.Connect(kDummyLoginToken);
+  base::Thread io_thread("BlimpIOThread");
+  base::Thread::Options options;
+  options.message_loop_type = base::MessageLoop::TYPE_IO;
+  io_thread.StartWithOptions(options);
+
+  // Creating this using "new" and passing to context using "WrapUnique" as

[Kevin M, 2016/09/27 21:59:41]

Move this declaration directly above the one for |context| to minimize the
distance between the place where it's allocated and the place where its lifetime
is attached.

[steimel, 2016/09/27 23:23:19]

Done.

+  // opposed to "MakeUnique" because we'll need to pass the compositor
+  // dependencies to the display manager as well.
+  blimp::client::CompositorDependencies* compositor_dependencies =
+      new blimp::client::CompositorDependenciesImpl();
+
+  std::unique_ptr<blimp::client::BlimpClientContextDelegate> context_delegate =

[Kevin M, 2016/09/27 21:59:41]

Put this just before SetDelegate, in the same semantic block.

General comment: try to cluster variable declarations as close to where they're
being used (so long as it still makes sense).

Use blank lines to separate discrete clusters.

[steimel, 2016/09/27 23:23:19]

Done. It's weird, but I could have sworn I had already done that.

[David Trainor- moved to gerrit, 2016/09/28 16:34:01]

I would undo this.  You don't want the delegate dying before the
BlimpClientContext.

[steimel, 2016/09/28 20:33:07]

Done and added comment to explain

+      base::MakeUnique<blimp::client::BlimpClientContextDelegateLinux>();
+
+  std::unique_ptr<blimp::client::BlimpClientContext> context =
+      base::WrapUnique<blimp::client::BlimpClientContext>(
+          blimp::client::BlimpClientContext::Create(
+              io_thread.task_runner(), io_thread.task_runner(),
+              base::WrapUnique(compositor_dependencies)));
+
+  context->SetDelegate(context_delegate.get());
+
+  context->Connect();
+
+  std::unique_ptr<blimp::client::BlimpContents> contents =

[Kevin M, 2016/09/27 21:59:41]

Move this right before GetNavigationController.

[steimel, 2016/09/27 23:23:19]

Done.

+      context->CreateBlimpContents(nullptr);
 
   // If there is a non-switch argument to the command line, load that url.
   base::CommandLine::StringVector args =
       base::CommandLine::ForCurrentProcess()->GetArgs();
   std::string url = args.size() > 0 ? args[0] : kDefaultUrl;
-  session.GetNavigationFeature()->NavigateToUrlText(kDummyTabId, url);
+  contents->GetNavigationController().LoadURL(GURL(url));
+
+  std::unique_ptr<ui::PlatformEventSource> event_source =

[Kevin M, 2016/09/27 21:59:41]

Is this used?

[steimel, 2016/09/27 23:23:19]

Good catch. I had meant to move this into the display manager, but it had
slipped through. You cannot create a new instance of an X11 window unless there
exists a PlatformEventSource. By creating one, it is set as static on the
PlatformEventSource class and used (that's why it currently works even though
it's within blimp_main). I've now moved this to be created and held entirely
within the display manager, since that's the object that actually cares about
it.

+      ui::PlatformEventSource::CreateDefault();
 
+  blimp::client::BlimpDisplayManager display_manager(
+      gfx::Size(kWindowWidth, kWindowHeight),
+      new blimp::client::BlimpDisplayManagerDelegateMain(),
+      compositor_dependencies, std::move(contents));
   base::RunLoop().Run();
 }