Revert 76747 - 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

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

TBR=simonmorris@chromium.org
Review URL: http://codereview.chromium.org/6610018

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

11 files changed, 73 insertions, 110 deletions

diff --git a/remoting/client/appengine/static_files/chromoting_session.js b/remoting/client/appengine/static_files/chromoting_session.js
index 3e22c85..9d4c481 100644
--- a/remoting/client/appengine/static_files/chromoting_session.js
+++ b/remoting/client/appengine/static_files/chromoting_session.js
@@ -24,7 +24,6 @@ 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 0cdfe62..eacd49e 100644
--- a/remoting/client/plugin/chromoting_instance.cc
+++ b/remoting/client/plugin/chromoting_instance.cc
@@ -73,8 +73,9 @@ bool ChromotingInstance::Init(uint32_t argc,
       context_.jingle_thread()));
   view_.reset(new PepperView(this, &context_));
   view_proxy_ = new PepperViewProxy(this, view_.get());
-  rectangle_decoder_ = new RectangleUpdateDecoder(
-      context_.decode_message_loop(), view_proxy_);
+  rectangle_decoder_.reset(
+      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 554760b..9e1c2d3 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_refptr<RectangleUpdateDecoder> rectangle_decoder_;
+  scoped_ptr<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 d1b7eec..b45b822 100644
--- a/remoting/client/plugin/chromoting_scriptable_object.cc
+++ b/remoting/client/plugin/chromoting_scriptable_object.cc
@@ -19,7 +19,6 @@ 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";
@@ -58,7 +57,6 @@ 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));
@@ -144,7 +142,6 @@ 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) {
@@ -211,7 +208,6 @@ 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.",
@@ -243,23 +239,6 @@ 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 9ebc8fa..b8b848e62 100644
--- a/remoting/client/plugin/chromoting_scriptable_object.h
+++ b/remoting/client/plugin/chromoting_scriptable_object.h
@@ -160,9 +160,6 @@ 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 6bc4508..5d154ba 100644
--- a/remoting/client/plugin/pepper_view.cc
+++ b/remoting/client/plugin/pepper_view.cc
@@ -20,6 +20,8 @@ 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),
@@ -81,8 +83,6 @@ 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,9 +178,11 @@ void PepperView::UpdateLoginStatus(bool success, const std::string& info) {
 void PepperView::SetViewport(int x, int y, int width, int height) {
   DCHECK(instance_->CurrentlyOnPluginThread());
 
-  if ((width == viewport_width_) && (height == viewport_height_))
-    return;
+  // TODO(ajwong): Should we ignore x & y updates?  What do those even mean?
 
+  // 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 119d1ca..f7822f3 100644
--- a/remoting/client/plugin/pepper_view.h
+++ b/remoting/client/plugin/pepper_view.h
@@ -16,6 +16,7 @@
 #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 {
 
@@ -67,6 +68,8 @@ 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 5659fc5..2690e14 100644
--- a/remoting/client/rectangle_update_decoder.cc
+++ b/remoting/client/rectangle_update_decoder.cc
@@ -44,16 +44,15 @@ class PartialFrameCleanup : public Task {
 RectangleUpdateDecoder::RectangleUpdateDecoder(MessageLoop* message_loop,
                                                FrameConsumer* consumer)
     : message_loop_(message_loop),
-      consumer_(consumer),
-      frame_is_new_(false) {
+      consumer_(consumer) {
 }
 
 RectangleUpdateDecoder::~RectangleUpdateDecoder() {
 }
 
 void RectangleUpdateDecoder::Initialize(const SessionConfig* config) {
-  initial_screen_size_ = gfx::Size(config->initial_resolution().width,
-                                   config->initial_resolution().height);
+  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;
@@ -88,90 +87,77 @@ void RectangleUpdateDecoder::DecodePacket(const VideoPacket* packet,
 
   TraceContext::tracer()->PrintString("Decode Packet called.");
 
-  AllocateFrame(packet, done_runner.release());
+  if (!decoder_->IsReadyForData()) {
+    InitializeDecoder(
+        NewTracedMethod(this,
+                        &RectangleUpdateDecoder::ProcessPacketData,
+                        packet, done_runner.release()));
+  } else {
+    ProcessPacketData(packet, done_runner.release());
+  }
 }
 
-void RectangleUpdateDecoder::AllocateFrame(const VideoPacket* packet,
-                                           Task* done) {
+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) {
   if (message_loop_ != MessageLoop::current()) {
     message_loop_->PostTask(
         FROM_HERE,
         NewTracedMethod(this,
-                        &RectangleUpdateDecoder::AllocateFrame, packet, done));
+                        &RectangleUpdateDecoder::InitializeDecoder, done));
     return;
   }
   AutoTaskRunner done_runner(done);
 
-  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))) {
+  // 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())) {
     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_,
-                             NewRunnableMethod(this,
-                                 &RectangleUpdateDecoder::ProcessPacketData,
-                                 packet, done_runner.release()));
-    frame_is_new_ = true;
+                             NewTracedMethod(
+                                 this,
+                                 &RectangleUpdateDecoder::InitializeDecoder,
+                                 done_runner.release()));
     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;
-  }
 
-  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;
-  }
+  // TODO(ajwong): We need to handle the allocator failing to create a frame
+  // and properly disable this class.
+  CHECK(frame_);
 
-  TraceContext::tracer()->PrintString("Executing Decode.");
+  decoder_->Reset();
+  decoder_->Initialize(frame_);
 
-  if (decoder_->DecodePacket(packet) == Decoder::DECODE_DONE) {
-    UpdatedRects* rects = new UpdatedRects();
-    decoder_->GetUpdatedRects(rects);
-    consumer_->OnPartialFrameOutput(frame_, rects,
-                                    new PartialFrameCleanup(frame_, rects));
-  }
+  TraceContext::tracer()->PrintString("Decoder is Initialized");
 }
 
 }  // namespace remoting
diff --git a/remoting/client/rectangle_update_decoder.h b/remoting/client/rectangle_update_decoder.h
index fbcc69b..bcd521a 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 :
-    public base::RefCountedThreadSafe<RectangleUpdateDecoder> {
+class 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,23 +45,20 @@ class RectangleUpdateDecoder :
   void DecodePacket(const VideoPacket* packet, Task* done);
 
  private:
-   friend class base::RefCountedThreadSafe<RectangleUpdateDecoder>;
-  ~RectangleUpdateDecoder();
+  void InitializeDecoder(Task* done);
 
-  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 initial_screen_size_;
+  gfx::Size screen_size_;
 
   scoped_ptr<Decoder> decoder_;
 
-  // The video frame that the decoder writes to.
+  // Framebuffer for the decoder.
   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 5d0867f..7bfd3b4 100644
--- a/remoting/client/x11_client.cc
+++ b/remoting/client/x11_client.cc
@@ -34,12 +34,11 @@ int main(int argc, char** argv) {
   remoting::ClientContext context;
   remoting::protocol::ConnectionToHost connection(context.jingle_thread());
   remoting::X11View view;
-  scoped_refptr<remoting::RectangleUpdateDecoder> rectangle_decoder =
-      new remoting::RectangleUpdateDecoder(context.decode_message_loop(),
-                                           &view);
+  remoting::RectangleUpdateDecoder rectangle_decoder(
+      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 d3e9623..9ba7fc1 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/chromoting_view.h"
 #include "remoting/client/frame_consumer.h"
+#include "remoting/client/chromoting_view.h"
 
 typedef unsigned long XID;
 typedef struct _XDisplay Display;