Various improvements to the Chromoting Android app

remoting/client/jni/jni_frame_consumer.cc

Index: remoting/client/jni/jni_frame_consumer.cc
diff --git a/remoting/client/jni/jni_frame_consumer.cc b/remoting/client/jni/jni_frame_consumer.cc
index ee8ef947c8c175f251ec97185d1f4e575842a232..965a8756691997423879df2f490f071c1424331a 100644
--- a/remoting/client/jni/jni_frame_consumer.cc
+++ b/remoting/client/jni/jni_frame_consumer.cc
@@ -8,7 +8,7 @@
 #include "base/logging.h"
 #include "base/synchronization/waitable_event.h"
 #include "remoting/client/frame_producer.h"
-#include "remoting/client/jni/chromoting_jni.h"
+#include "remoting/client/jni/chromoting_jni_runtime.h"
 #include "third_party/webrtc/modules/desktop_capture/desktop_frame.h"
 
 namespace {
@@ -41,14 +41,15 @@ DirectDesktopFrame::~DirectDesktopFrame() {}
 
 namespace remoting {
 
-JniFrameConsumer::JniFrameConsumer()
-    : provide_buffer_(true),
-      frame_producer_(NULL) {
+JniFrameConsumer::JniFrameConsumer(ChromotingJniRuntime* jni_environment)
+    : in_dtor_(false),
+      frame_producer_(NULL),
+      jni_runtime_(jni_environment) {
 }
 
 JniFrameConsumer::~JniFrameConsumer() {
   // Stop giving the producer a buffer to work with.
-  provide_buffer_ = false;
+  in_dtor_ = true;
 
   // Don't destroy the object until we've deleted the buffer.
   base::WaitableEvent done_event(true, false);
@@ -65,35 +66,33 @@ void JniFrameConsumer::ApplyBuffer(const SkISize& view_size,
                                    const SkIRect& clip_area,
                                    webrtc::DesktopFrame* buffer,
                                    const SkRegion& region) {
-  DCHECK(ChromotingJni::GetInstance()->
-      display_task_runner()->BelongsToCurrentThread());
+  DCHECK(jni_runtime_->display_task_runner()->BelongsToCurrentThread());
 
-  ChromotingJni::GetInstance()->RedrawCanvas();
+  scoped_ptr<webrtc::DesktopFrame> buffer_scoped(buffer);
+  jni_runtime_->RedrawCanvas();
 
   if (view_size.width() > view_size_.width() ||
       view_size.height() > view_size_.height()) {
     LOG(INFO) << "Existing buffer is too small";
     view_size_ = view_size;
-    delete buffer;
+    buffer_scoped.reset();

[Wez, 2013/07/23 19:59:07]

nit: Add a comment to explin why you do this rather than let scoped_ptr<> clean
it up when it goes out of scope

[solb, 2013/07/23 20:50:28]

Now that you mention it, I guess I'll do just that. ;)

[Wez, 2013/07/23 20:53:01]

Oh, I assumed it was deliberate, to avoid temporarily hosing memory by having
two buffers allocated?

     AllocateBuffer();
   }
 
   // Supply |frame_producer_| with a buffer to render the next frame into.
-  if (provide_buffer_)
-    frame_producer_->DrawBuffer(buffer);
+  if (!in_dtor_)
+    frame_producer_->DrawBuffer(buffer_scoped.release());
 }
 
 void JniFrameConsumer::ReturnBuffer(webrtc::DesktopFrame* buffer) {
-  DCHECK(ChromotingJni::GetInstance()->
-      display_task_runner()->BelongsToCurrentThread());
+  DCHECK(jni_runtime_->display_task_runner()->BelongsToCurrentThread());
   LOG(INFO) << "Returning image buffer";
   delete buffer;
 }
 
 void JniFrameConsumer::SetSourceSize(const SkISize& source_size,
                                      const SkIPoint& dpi) {
-  DCHECK(ChromotingJni::GetInstance()->
-      display_task_runner()->BelongsToCurrentThread());
+  DCHECK(jni_runtime_->display_task_runner()->BelongsToCurrentThread());
 
   // We currently render the desktop 1:1 and perform pan/zoom scaling
   // and cropping on the managed canvas.
@@ -108,10 +107,9 @@ void JniFrameConsumer::SetSourceSize(const SkISize& source_size,
 
 void JniFrameConsumer::AllocateBuffer() {
   // Only do anything if we're not being destructed.
-  if (provide_buffer_) {
-    if (!ChromotingJni::GetInstance()->
-        display_task_runner()->BelongsToCurrentThread()) {
-      ChromotingJni::GetInstance()->display_task_runner()->PostTask(FROM_HERE,
+  if (!in_dtor_) {
+    if (!jni_runtime_->display_task_runner()->BelongsToCurrentThread()) {
+      jni_runtime_->display_task_runner()->PostTask(FROM_HERE,
           base::Bind(&JniFrameConsumer::AllocateBuffer,
                      base::Unretained(this)));
       return;
@@ -121,10 +119,9 @@ void JniFrameConsumer::AllocateBuffer() {
                                                         view_size_.height());
 
     // Update Java's reference to the buffer and record of its dimensions.
-    ChromotingJni::GetInstance()->UpdateImageBuffer(
-        view_size_.width(),
-        view_size_.height(),
-        buffer->buffer());
+    jni_runtime_->UpdateImageBuffer(view_size_.width(),
+                                        view_size_.height(),
+                                        buffer->buffer());
 
     frame_producer_->DrawBuffer(buffer);
   }