[Remoting Android] DisplayUpdaterFactory cleanups

remoting/client/jni/jni_display_handler.cc

 // 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.
 
 #include "remoting/client/jni/jni_display_handler.h"
 
 #include "base/android/jni_android.h"
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/memory/ptr_util.h"
 #include "base/stl_util.h"
 #include "jni/Display_jni.h"
 #include "remoting/client/jni/chromoting_jni_runtime.h"
 #include "remoting/client/jni/jni_client.h"
-#include "remoting/client/jni/jni_frame_consumer.h"
 #include "remoting/client/jni/jni_video_renderer.h"
 
 using base::android::JavaParamRef;
 
 namespace {
 
 const int kBytesPerPixel = 4;
 
 }
 
@@ skipping 55 matching lines @@
       env, env->NewDirectByteBuffer(data, cursor_total_bytes));
   Java_Display_updateCursorShape(
       env, java_display_.obj(), cursor_shape.width(), cursor_shape.height(),
       cursor_shape.hotspot_x(), cursor_shape.hotspot_y(), buffer.obj());
 }
 
 std::unique_ptr<protocol::CursorShapeStub>
 JniDisplayHandler::CreateCursorShapeStub() {
   return base::WrapUnique(
       new DisplayCursorShapeStub(weak_factory_.GetWeakPtr(),
-                                    runtime_->display_task_runner()));
+                                 runtime_->display_task_runner()));
 }
 
-std::unique_ptr<JniVideoRenderer> JniDisplayHandler::CreateVideoRenderer() {
+std::unique_ptr<protocol::VideoRenderer>
+JniDisplayHandler::CreateVideoRenderer() {
   return base::WrapUnique(
-      new JniFrameConsumer(runtime_, weak_factory_.GetWeakPtr()));
+      new JniVideoRenderer(runtime_, weak_factory_.GetWeakPtr()));

[Yuwei, 2016/06/30 07:51:02]

After separating initialization code from the ctor of SoftwareVideoRenderer,
looks like we don't even need to make JniFrameConsumer a video renderer, and
here we can simply return an uninitialized software renderer like

return base::WrapUnique(new SoftwareVideoRenderer(frame_consumer));

The slight problem is that it doesn't take ownership of the frame consumer and
JniDisplayHandler will need to delete the consumer on the network thread.

[Yuwei, 2016/06/30 19:13:54]

I will hold off this change to a future CL. Currently SoftwareVideoRenderer
takes in a raw pointer so it must not outlive the underlying frame consumer,
while the user of DisplayUpdaterFactory doesn't need to know anything about the
underlying frame consumer. Maybe later we can make SoftwareVideoRenderer take in
WeakPtr of frame consumer but that will involve lots of code changes of existing
frame consumers...

 }
 
 // static
 base::android::ScopedJavaLocalRef<jobject> JniDisplayHandler::NewBitmap(
     int width,
     int height) {
   JNIEnv* env = base::android::AttachCurrentThread();
   return Java_Display_newBitmap(env, width, height);
 }
 
@@ skipping 35 matching lines @@
                             weak_factory_.GetWeakPtr()));
 }
 
 static jlong Init(JNIEnv* env, const JavaParamRef<jobject>& caller) {
   return reinterpret_cast<intptr_t>(new JniDisplayHandler(
       ChromotingJniRuntime::GetInstance(),
       base::android::ScopedJavaGlobalRef<jobject>(env, caller)));
 }
 
 }  // namespace remoting