Move glue code of blob channel to blimp_client_context_impl.

blimp/client/core/blimp_client_context_impl.cc

Index: blimp/client/core/blimp_client_context_impl.cc
diff --git a/blimp/client/core/blimp_client_context_impl.cc b/blimp/client/core/blimp_client_context_impl.cc
index 009c15107db20ac43d6f0cce13302fb02a91d964..7f2b2453c212bd0c66cfd04ca323709724882e48 100644
--- a/blimp/client/core/blimp_client_context_impl.cc
+++ b/blimp/client/core/blimp_client_context_impl.cc
@@ -8,6 +8,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/threading/sequenced_task_runner_handle.h"
+#include "blimp/client/core/compositor/decoding_image_generator.h"
 #include "blimp/client/core/contents/blimp_contents_impl.h"
 #include "blimp/client/core/contents/blimp_contents_manager.h"
 #include "blimp/client/core/contents/ime_feature.h"
@@ -15,6 +16,10 @@
 #include "blimp/client/core/contents/tab_control_feature.h"
 #include "blimp/client/core/session/cross_thread_network_event_observer.h"
 #include "blimp/client/public/blimp_client_context_delegate.h"
+#include "blimp/common/blob_cache/in_memory_blob_cache.h"
+#include "blimp/net/blob_channel/blob_channel_receiver.h"
+#include "blimp/net/blob_channel/helium_blob_receiver_delegate.h"
+#include "third_party/skia/include/core/SkGraphics.h"
 
 #if defined(OS_ANDROID)
 #include "blimp/client/core/android/blimp_client_context_impl_android.h"
@@ -24,10 +29,20 @@ namespace blimp {
 namespace client {
 
 namespace {
+
+SkImageGenerator* CreateImageGenerator(SkData* data) {
+  return blimp::client::DecodingImageGenerator::create(data);
+}
+
 const char kDefaultAssignerUrl[] =
     "https://blimp-pa.googleapis.com/v1/assignment";
 }  // namespace
 
+// static
+void BlimpClientContextImpl::SetImageGeneratorFromEncodedFactory() {
+  SkGraphics::SetImageGeneratorFromEncodedFactory(CreateImageGenerator);
+}
+
 // This function is declared in //blimp/client/public/blimp_client_context.h,
 // and either this function or the one in
 // //blimp/client/core/dummy_blimp_client_context.cc should be linked in to
@@ -68,6 +83,8 @@ BlimpClientContextImpl::BlimpClientContextImpl(
   thread_pipe_manager_ = base::MakeUnique<ThreadPipeManager>(
       io_thread_task_runner_, net_components_->GetBrowserConnectionHandler());
 
+  SetupBlobChannel();
+
   RegisterFeatures();
 
   // Initialize must only be posted after the calls features have been
@@ -75,6 +92,8 @@ BlimpClientContextImpl::BlimpClientContextImpl(
   io_thread_task_runner_->PostTask(
       FROM_HERE, base::Bind(&ClientNetworkComponents::Initialize,
                             base::Unretained(net_components_.get())));
+
+  SetImageGeneratorFromEncodedFactory();

[Khushal, 2016/08/31 03:55:08]

I checked, we don't need to do this. skia calls out to the static only if you
don't give a image deserializer when deserializing the SkPicture, which we
always provide. So you can avoid adding it. :)

[David Trainor- moved to gerrit, 2016/08/31 05:40:57]

Looked through the code quickly as well and it looks correct.  w00t this would
be great.

[nyquist, 2016/09/01 00:26:23]

I wonder, are there any times where we have to re-decode the data? I vaguely
remember me having this here because they all of a sudden called us
"out-of-the-blue", where we did not get the option to pass it in. But I can't
seem to remember when that happened, or whether it will still happen.

[xingliu, 2016/09/01 02:08:09]

I'll remove this.

 }
 
 BlimpClientContextImpl::~BlimpClientContextImpl() {
@@ -163,6 +182,26 @@ void BlimpClientContextImpl::RegisterFeatures() {
   tab_control_feature_->set_outgoing_message_processor(
       thread_pipe_manager_->RegisterFeature(BlimpMessage::kTabControl,
                                             tab_control_feature_.get()));
+  thread_pipe_manager_->RegisterFeature(BlimpMessage::kBlobChannel,

[David Trainor- moved to gerrit, 2016/08/31 05:40:57]

let's try to keep these in alphabetical order for the feature?

[xingliu, 2016/09/01 02:08:10]

Done.

+                                        blob_delegate_);
+}
+
+void BlimpClientContextImpl::SetupBlobChannel() {
+  std::unique_ptr<HeliumBlobReceiverDelegate> blob_delegate(
+      new HeliumBlobReceiverDelegate);

[Khushal, 2016/08/31 03:55:08]

nit: prefer MakeUnique over new Foo. For this and in the receiver below.

[xingliu, 2016/09/01 02:08:09]

Changed the first one. Didn't change the second one to use MakeUnique since it
passed base type pointer.

+  blob_delegate_ = blob_delegate.get();

[Khushal, 2016/08/31 03:55:08]

Do we actually need to cache this pointer? We only use it to register the
feature, so would it be cleaner to do the thread_pipe_manager_->RegisterFeature
for blob_delegate in SetupBlobChannel itself?

[David Trainor- moved to gerrit, 2016/08/31 05:40:57]

Would it be cleaner to expose a BlobChannelReceiver::delegate() method?  That
would make it so that we can still keep all of the registration together in the
same method.

[xingliu, 2016/09/01 02:08:10]

RegisterFeatures needs BlimpMessageProcessor*, HeliumBlobReceiverDelegate
implements BlimpMessageProcessor and BlobChannelReceiver::Delegate, so if we
doing this, need to static cast. Should I do it here?

Should I wrap these objects into class like BlobChannelFeature? so it looks
similar to other stuff. Right now these things are blimp/net code.

[Khushal, 2016/09/01 02:10:10]

Yeah, that's cool too. Or just return the receiver from this method and pass it
to the RegisterFeatures method.

But why do we need the delegate interface at all? All the delegate is used for
is to set the receiver, and feels like the code here should be able to hook that
up? For lifetimes, you can make sure that you destroy the receiver before
destroying the Helium object?

[xingliu, 2016/09/01 21:03:34]

For test, they have a mock message processor.

+  blob_receiver_ = BlobChannelReceiver::Create(
+      base::WrapUnique(new InMemoryBlobCache), std::move(blob_delegate));
+  blob_image_processor_.set_blob_receiver(blob_receiver_.get());
+  blob_image_processor_.set_error_delegate(this);
+}
+
+void BlimpClientContextImpl::OnImageDecodeError() {
+  io_thread_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(
+          &BrowserConnectionHandler::DropCurrentConnection,
+          base::Unretained(net_components_->GetBrowserConnectionHandler())));
 }
 
 void BlimpClientContextImpl::CreateIdentitySource() {