Let the host change resolution.

The screen size flows through the video pipeline, instead of
being set statically when that pipeline is constructed.

Only the Windows host actually detects when the screen size has changed.

BUG=72469
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=76747

Review URL: http://codereview.chromium.org/6573005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@76908 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 110 insertions, 73 deletions

diff --git a/remoting/client/appengine/static_files/chromoting_session.js b/remoting/client/appengine/static_files/chromoting_session.js
index 9d4c481..3e22c85 100644
--- a/remoting/client/appengine/static_files/chromoting_session.js
+++ b/remoting/client/appengine/static_files/chromoting_session.js
@@ -24,6 +24,7 @@ function init() {
   // a 'callback' property that contains the callback function.
   plugin.connectionInfoUpdate = connectionInfoUpdateCallback;
   plugin.debugInfo = debugInfoCallback;
+  plugin.desktopSizeUpdate = desktopSizeChanged;
   plugin.loginChallenge = loginChallengeCallback;
 
   console.log('connect request received: ' + chromoting.hostname + ' by ' +
diff --git a/remoting/client/plugin/chromoting_instance.cc b/remoting/client/plugin/chromoting_instance.cc
index eacd49e..0cdfe62 100644
--- a/remoting/client/plugin/chromoting_instance.cc
+++ b/remoting/client/plugin/chromoting_instance.cc
@@ -73,9 +73,8 @@ bool ChromotingInstance::Init(uint32_t argc,
       context_.jingle_thread()));
   view_.reset(new PepperView(this, &context_));
   view_proxy_ = new PepperViewProxy(this, view_.get());
-  rectangle_decoder_.reset(
-      new RectangleUpdateDecoder(context_.decode_message_loop(),
-                                 view_proxy_));
+  rectangle_decoder_ = new RectangleUpdateDecoder(
+      context_.decode_message_loop(), view_proxy_);
   input_handler_.reset(new PepperInputHandler(&context_,
                                               host_connection_.get(),
                                               view_proxy_));
diff --git a/remoting/client/plugin/chromoting_instance.h b/remoting/client/plugin/chromoting_instance.h
index 9e1c2d3..554760b 100644
--- a/remoting/client/plugin/chromoting_instance.h
+++ b/remoting/client/plugin/chromoting_instance.h
@@ -101,7 +101,7 @@ class ChromotingInstance : public pp::Instance {
   // both ChromotingInstance and PepperView are destroyed and there will be
   // outstanding tasks on the pepper message loo.
   scoped_refptr<PepperViewProxy> view_proxy_;
-  scoped_ptr<RectangleUpdateDecoder> rectangle_decoder_;
+  scoped_refptr<RectangleUpdateDecoder> rectangle_decoder_;
   scoped_ptr<InputHandler> input_handler_;
   scoped_ptr<ChromotingClient> client_;
 
diff --git a/remoting/client/plugin/chromoting_scriptable_object.cc b/remoting/client/plugin/chromoting_scriptable_object.cc
index b45b822..d1b7eec 100644
--- a/remoting/client/plugin/chromoting_scriptable_object.cc
+++ b/remoting/client/plugin/chromoting_scriptable_object.cc
@@ -19,6 +19,7 @@ const char kConnectionInfoUpdate[] = "connectionInfoUpdate";
 const char kDebugInfo[] = "debugInfo";
 const char kDesktopHeight[] = "desktopHeight";
 const char kDesktopWidth[] = "desktopWidth";
+const char kDesktopSizeUpdate[] = "desktopSizeUpdate";
 const char kLoginChallenge[] = "loginChallenge";
 const char kQualityAttribute[] = "quality";
 const char kStatusAttribute[] = "status";
@@ -57,6 +58,7 @@ void ChromotingScriptableObject::Init() {
   // Debug info to display.
   AddAttribute(kConnectionInfoUpdate, Var());
   AddAttribute(kDebugInfo, Var());
+  AddAttribute(kDesktopSizeUpdate, Var());
   AddAttribute(kLoginChallenge, Var());
   AddAttribute(kDesktopWidth, Var(0));
   AddAttribute(kDesktopHeight, Var(0));
@@ -142,6 +144,7 @@ void ChromotingScriptableObject::SetProperty(const Var& name,
   std::string property_name = name.AsString();
   if (property_name != kConnectionInfoUpdate &&
       property_name != kDebugInfo &&
+      property_name != kDesktopSizeUpdate &&
       property_name != kLoginChallenge &&
       property_name != kDesktopWidth &&
       property_name != kDesktopHeight) {
@@ -208,6 +211,7 @@ void ChromotingScriptableObject::SetDesktopSize(int width, int height) {
       properties_[height_index].attribute.AsInt() != height) {
     properties_[width_index].attribute = Var(width);
     properties_[height_index].attribute = Var(height);
+    SignalDesktopSizeChange();
   }
 
   LogDebugInfo(base::StringPrintf("Update desktop size to: %d x %d.",
@@ -239,6 +243,23 @@ void ChromotingScriptableObject::SignalConnectionInfoChange() {
         "Exception when invoking connectionInfoUpdate JS callback.");
 }
 
+void ChromotingScriptableObject::SignalDesktopSizeChange() {
+  Var exception;
+
+  // The JavaScript callback function is the 'callback' property on the
+  // 'desktopSizeUpdate' object.
+  Var cb = GetProperty(Var(kDesktopSizeUpdate), &exception);
+
+  // Var() means call the object directly as a function rather than calling
+  // a method in the object.
+  cb.Call(Var(), 0, NULL, &exception);
+
+  if (!exception.is_undefined()) {
+    LOG(WARNING) << "Exception when invoking JS callback"
+                 << exception.AsString();
+  }
+}
+
 void ChromotingScriptableObject::SignalLoginChallenge() {
   Var exception;
   Var cb = GetProperty(Var(kLoginChallenge), &exception);
diff --git a/remoting/client/plugin/chromoting_scriptable_object.h b/remoting/client/plugin/chromoting_scriptable_object.h
index b8b848e62..9ebc8fa 100644
--- a/remoting/client/plugin/chromoting_scriptable_object.h
+++ b/remoting/client/plugin/chromoting_scriptable_object.h
@@ -160,6 +160,9 @@ class ChromotingScriptableObject
   // changed.
   void SignalConnectionInfoChange();
 
+  // Signal the JS code that the desktop size has changed.
+  void SignalDesktopSizeChange();
+
   pp::Var DoConnect(const std::vector<pp::Var>& args, pp::Var* exception);
   pp::Var DoDisconnect(const std::vector<pp::Var>& args, pp::Var* exception);
 
diff --git a/remoting/client/plugin/pepper_view.cc b/remoting/client/plugin/pepper_view.cc
index 5d154ba..6bc4508 100644
--- a/remoting/client/plugin/pepper_view.cc
+++ b/remoting/client/plugin/pepper_view.cc
@@ -20,8 +20,6 @@ namespace remoting {
 PepperView::PepperView(ChromotingInstance* instance, ClientContext* context)
   : instance_(instance),
     context_(context),
-    viewport_x_(0),
-    viewport_y_(0),
     viewport_width_(0),
     viewport_height_(0),
     is_static_fill_(false),
@@ -83,6 +81,8 @@ void PepperView::PaintFrame(media::VideoFrame* frame, UpdatedRects* rects) {
 
   TraceContext::tracer()->PrintString("Start Paint Frame.");
 
+  SetViewport(0, 0, frame->width(), frame->height());
+
   uint8* frame_data = frame->data(media::VideoFrame::kRGBPlane);
   const int kFrameStride = frame->stride(media::VideoFrame::kRGBPlane);
   const int kBytesPerPixel = GetBytesPerPixel(media::VideoFrame::RGB32);
@@ -178,11 +178,9 @@ void PepperView::UpdateLoginStatus(bool success, const std::string& info) {
 void PepperView::SetViewport(int x, int y, int width, int height) {
   DCHECK(instance_->CurrentlyOnPluginThread());
 
-  // TODO(ajwong): Should we ignore x & y updates?  What do those even mean?
+  if ((width == viewport_width_) && (height == viewport_height_))
+    return;
 
-  // TODO(ajwong): What does viewport x, y mean to a plugin anyways?
-  viewport_x_ = x;
-  viewport_y_ = y;
   viewport_width_ = width;
   viewport_height_ = height;
 
diff --git a/remoting/client/plugin/pepper_view.h b/remoting/client/plugin/pepper_view.h
index f7822f3..119d1ca 100644
--- a/remoting/client/plugin/pepper_view.h
+++ b/remoting/client/plugin/pepper_view.h
@@ -16,7 +16,6 @@
 #include "ppapi/cpp/graphics_2d.h"
 #include "remoting/client/chromoting_view.h"
 #include "remoting/client/frame_consumer.h"
-#include "remoting/client/rectangle_update_decoder.h"
 
 namespace remoting {
 
@@ -68,8 +67,6 @@ class PepperView : public ChromotingView,
 
   pp::Graphics2D graphics2d_;
 
-  int viewport_x_;
-  int viewport_y_;
   int viewport_width_;
   int viewport_height_;
 
diff --git a/remoting/client/rectangle_update_decoder.cc b/remoting/client/rectangle_update_decoder.cc
index 2690e14..5659fc5 100644
--- a/remoting/client/rectangle_update_decoder.cc
+++ b/remoting/client/rectangle_update_decoder.cc
@@ -44,15 +44,16 @@ class PartialFrameCleanup : public Task {
 RectangleUpdateDecoder::RectangleUpdateDecoder(MessageLoop* message_loop,
                                                FrameConsumer* consumer)
     : message_loop_(message_loop),
-      consumer_(consumer) {
+      consumer_(consumer),
+      frame_is_new_(false) {
 }
 
 RectangleUpdateDecoder::~RectangleUpdateDecoder() {
 }
 
 void RectangleUpdateDecoder::Initialize(const SessionConfig* config) {
-  screen_size_ = gfx::Size(config->initial_resolution().width,
-                           config->initial_resolution().height);
+  initial_screen_size_ = gfx::Size(config->initial_resolution().width,
+                                   config->initial_resolution().height);
 
   // Initialize decoder based on the selected codec.
   ChannelConfig::Codec codec = config->video_config().codec;
@@ -87,77 +88,90 @@ void RectangleUpdateDecoder::DecodePacket(const VideoPacket* packet,
 
   TraceContext::tracer()->PrintString("Decode Packet called.");
 
-  if (!decoder_->IsReadyForData()) {
-    InitializeDecoder(
-        NewTracedMethod(this,
-                        &RectangleUpdateDecoder::ProcessPacketData,
-                        packet, done_runner.release()));
-  } else {
-    ProcessPacketData(packet, done_runner.release());
-  }
+  AllocateFrame(packet, done_runner.release());
 }
 
-void RectangleUpdateDecoder::ProcessPacketData(
-    const VideoPacket* packet, Task* done) {
-  AutoTaskRunner done_runner(done);
-
-  if (!decoder_->IsReadyForData()) {
-    // TODO(ajwong): This whole thing should move into an invalid state.
-    LOG(ERROR) << "Decoder is unable to process data. Dropping packet.";
-    return;
-  }
-
-  TraceContext::tracer()->PrintString("Executing Decode.");
-
-  Decoder::DecodeResult result = decoder_->DecodePacket(packet);
-
-  if (result == Decoder::DECODE_DONE) {
-    UpdatedRects* rects = new UpdatedRects();
-    decoder_->GetUpdatedRects(rects);
-    consumer_->OnPartialFrameOutput(frame_, rects,
-                                    new PartialFrameCleanup(frame_, rects));
-  }
-}
-
-void RectangleUpdateDecoder::InitializeDecoder(Task* done) {
+void RectangleUpdateDecoder::AllocateFrame(const VideoPacket* packet,
+                                           Task* done) {
   if (message_loop_ != MessageLoop::current()) {
     message_loop_->PostTask(
         FROM_HERE,
         NewTracedMethod(this,
-                        &RectangleUpdateDecoder::InitializeDecoder, done));
+                        &RectangleUpdateDecoder::AllocateFrame, packet, done));
     return;
   }
   AutoTaskRunner done_runner(done);
 
-  // Check if we need to request a new frame.
-  if (!frame_ ||
-      frame_->width() != static_cast<size_t>(screen_size_.width()) ||
-      frame_->height() != static_cast<size_t>(screen_size_.height())) {
+  TraceContext::tracer()->PrintString("AllocateFrame called.");
+
+  // Find the required frame size.
+  bool has_screen_size = packet->format().has_screen_width() &&
+                         packet->format().has_screen_height();
+  gfx::Size screen_size(packet->format().screen_width(),
+                        packet->format().screen_height());
+  if (!has_screen_size)
+    screen_size = initial_screen_size_;
+
+  // Find the current frame size.
+  gfx::Size frame_size(0, 0);
+  if (frame_)
+    frame_size = gfx::Size(static_cast<int>(frame_->width()),
+                           static_cast<int>(frame_->height()));
+
+  // Allocate a new frame, if necessary.
+  if ((!frame_) || (has_screen_size && (screen_size != frame_size))) {
     if (frame_) {
       TraceContext::tracer()->PrintString("Releasing old frame.");
       consumer_->ReleaseFrame(frame_);
       frame_ = NULL;
     }
     TraceContext::tracer()->PrintString("Requesting new frame.");
+
     consumer_->AllocateFrame(media::VideoFrame::RGB32,
-                             screen_size_.width(), screen_size_.height(),
+                             screen_size.width(), screen_size.height(),
                              base::TimeDelta(), base::TimeDelta(),
                              &frame_,
-                             NewTracedMethod(
-                                 this,
-                                 &RectangleUpdateDecoder::InitializeDecoder,
-                                 done_runner.release()));
+                             NewRunnableMethod(this,
+                                 &RectangleUpdateDecoder::ProcessPacketData,
+                                 packet, done_runner.release()));
+    frame_is_new_ = true;
     return;
   }
+  ProcessPacketData(packet, done_runner.release());
+}
+
+void RectangleUpdateDecoder::ProcessPacketData(
+    const VideoPacket* packet, Task* done) {
+  if (message_loop_ != MessageLoop::current()) {
+    message_loop_->PostTask(
+        FROM_HERE,
+        NewTracedMethod(this,
+                        &RectangleUpdateDecoder::ProcessPacketData, packet,
+                        done));
+    return;
+  }
+  AutoTaskRunner done_runner(done);
+
+  if (frame_is_new_) {
+    decoder_->Reset();
+    decoder_->Initialize(frame_);
+    frame_is_new_ = false;
+  }
 
-  // TODO(ajwong): We need to handle the allocator failing to create a frame
-  // and properly disable this class.
-  CHECK(frame_);
+  if (!decoder_->IsReadyForData()) {
+    // TODO(ajwong): This whole thing should move into an invalid state.
+    LOG(ERROR) << "Decoder is unable to process data. Dropping packet.";
+    return;
+  }
 
-  decoder_->Reset();
-  decoder_->Initialize(frame_);
+  TraceContext::tracer()->PrintString("Executing Decode.");
 
-  TraceContext::tracer()->PrintString("Decoder is Initialized");
+  if (decoder_->DecodePacket(packet) == Decoder::DECODE_DONE) {
+    UpdatedRects* rects = new UpdatedRects();
+    decoder_->GetUpdatedRects(rects);
+    consumer_->OnPartialFrameOutput(frame_, rects,
+                                    new PartialFrameCleanup(frame_, rects));
+  }
 }
 
 }  // namespace remoting
diff --git a/remoting/client/rectangle_update_decoder.h b/remoting/client/rectangle_update_decoder.h
index bcd521a..fbcc69b 100644
--- a/remoting/client/rectangle_update_decoder.h
+++ b/remoting/client/rectangle_update_decoder.h
@@ -8,13 +8,13 @@
 #include "base/scoped_ptr.h"
 #include "base/task.h"
 #include "media/base/video_frame.h"
+#include "remoting/base/decoder.h"
 #include "ui/gfx/size.h"
 
 class MessageLoop;
 
 namespace remoting {
 
-class Decoder;
 class FrameConsumer;
 class VideoPacketFormat;
 class VideoPacket;
@@ -26,11 +26,11 @@ class SessionConfig;
 // TODO(ajwong): Re-examine this API, especially with regards to how error
 // conditions on each step are reported.  Should they be CHECKs? Logs? Other?
 // TODO(sergeyu): Rename this class.
-class RectangleUpdateDecoder {
+class RectangleUpdateDecoder :
+    public base::RefCountedThreadSafe<RectangleUpdateDecoder> {
  public:
   RectangleUpdateDecoder(MessageLoop* message_loop,
                          FrameConsumer* consumer);
-  ~RectangleUpdateDecoder();
 
   // Initializes decoder with the infromation from the protocol config.
   void Initialize(const protocol::SessionConfig* config);
@@ -45,20 +45,23 @@ class RectangleUpdateDecoder {
   void DecodePacket(const VideoPacket* packet, Task* done);
 
  private:
-  void InitializeDecoder(Task* done);
+   friend class base::RefCountedThreadSafe<RectangleUpdateDecoder>;
+  ~RectangleUpdateDecoder();
 
+  void AllocateFrame(const VideoPacket* packet, Task* done);
   void ProcessPacketData(const VideoPacket* packet, Task* done);
 
   // Pointers to infrastructure objects.  Not owned.
   MessageLoop* message_loop_;
   FrameConsumer* consumer_;
 
-  gfx::Size screen_size_;
+  gfx::Size initial_screen_size_;
 
   scoped_ptr<Decoder> decoder_;
 
-  // Framebuffer for the decoder.
+  // The video frame that the decoder writes to.
   scoped_refptr<media::VideoFrame> frame_;
+  bool frame_is_new_;
 };
 
 }  // namespace remoting
diff --git a/remoting/client/x11_client.cc b/remoting/client/x11_client.cc
index 7bfd3b4..5d0867f 100644
--- a/remoting/client/x11_client.cc
+++ b/remoting/client/x11_client.cc
@@ -34,11 +34,12 @@ int main(int argc, char** argv) {
   remoting::ClientContext context;
   remoting::protocol::ConnectionToHost connection(context.jingle_thread());
   remoting::X11View view;
-  remoting::RectangleUpdateDecoder rectangle_decoder(
-      context.decode_message_loop(), &view);
+  scoped_refptr<remoting::RectangleUpdateDecoder> rectangle_decoder =
+      new remoting::RectangleUpdateDecoder(context.decode_message_loop(),
+                                           &view);
   remoting::X11InputHandler input_handler(&context, &connection, &view);
   remoting::ChromotingClient client(
-      config, &context, &connection, &view, &rectangle_decoder, &input_handler,
+      config, &context, &connection, &view, rectangle_decoder, &input_handler,
       NewRunnableFunction(&ClientQuit, &ui_loop));
 
   // Run the client on a new MessageLoop until
diff --git a/remoting/client/x11_view.h b/remoting/client/x11_view.h
index 9ba7fc1..d3e9623 100644
--- a/remoting/client/x11_view.h
+++ b/remoting/client/x11_view.h
@@ -9,8 +9,8 @@
 #include "base/task.h"
 #include "media/base/video_frame.h"
 #include "remoting/base/decoder.h"  // For UpdatedRects
-#include "remoting/client/frame_consumer.h"
 #include "remoting/client/chromoting_view.h"
+#include "remoting/client/frame_consumer.h"
 
 typedef unsigned long XID;
 typedef struct _XDisplay Display;