Reapply 53892 with ARM build fix as suggested by Kobic.

For some reason, the ARM g++ thinks that comparing a pointer to a class
method against NULL is using NULL for arithemtic.  Switching to compare
against 0 makes this go away.  TODO added to investigate what is going on.

Original Review URL: http://codereview.chromium.org/3064009

TBR: hclam@chromium.org
BUG=50248
TEST=reproed locally with CodeSourcery g++ and verified change fixed things.

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

10 files changed, 334 insertions, 107 deletions

diff --git a/remoting/client/client_util.cc b/remoting/client/client_util.cc
index 215ae9c..28bd7cc 100644
--- a/remoting/client/client_util.cc
+++ b/remoting/client/client_util.cc
@@ -78,42 +78,4 @@ bool GetLoginInfoFromArgs(int argc, char** argv, ClientConfig* config) {
   return true;
 }
 
-// Get host JID from command line arguments, or stdin if not specified.
-bool GetLoginInfoFromUrlParams(const std::string& url, ClientConfig* config) {
-  // TODO(ajwong): We should use GURL or something.  Don't parse this by hand!
-
-  // The Url should be of the form:
-  //
-  //   chrome://remoting?user=<userid>&auth=<authtoken>&jid=<hostjid>
-  //
-  vector<string> parts;
-  SplitString(url, '&', &parts);
-  if (parts.size() != 3) {
-    return false;
-  }
-
-  size_t pos = parts[0].rfind('=');
-  if (pos == string::npos && (pos + 1) != string::npos) {
-    return false;
-  }
-  std::string username = parts[0].substr(pos + 1);
-
-  pos = parts[1].rfind('=');
-  if (pos == string::npos && (pos + 1) != string::npos) {
-    return false;
-  }
-  std::string auth_token = parts[1].substr(pos + 1);
-
-  pos = parts[2].rfind('=');
-  if (pos == string::npos && (pos + 1) != string::npos) {
-    return false;
-  }
-  std::string host_jid = parts[2].substr(pos + 1);
-
-  config->host_jid = host_jid;
-  config->username = username;
-  config->auth_token = auth_token;
-  return true;
-}
-
 }  // namespace remoting
diff --git a/remoting/client/client_util.h b/remoting/client/client_util.h
index 8cad1d5..3660563 100644
--- a/remoting/client/client_util.h
+++ b/remoting/client/client_util.h
@@ -16,10 +16,6 @@ struct ClientConfig;
 // Return true if successful.
 bool GetLoginInfoFromArgs(int argc, char** argv, ClientConfig* config);
 
-// Get the login info from the URL params and write values into |config|.
-// Return true if successful.
-bool GetLoginInfoFromUrlParams(const std::string& url, ClientConfig* config);
-
 }  // namespace remoting
 
 #endif  // REMOTING_CLIENT_CLIENT_UTIL_H_
diff --git a/remoting/client/client_util_unittest.cc b/remoting/client/client_util_unittest.cc
index 4277769..b1c3d21 100644
--- a/remoting/client/client_util_unittest.cc
+++ b/remoting/client/client_util_unittest.cc
@@ -10,26 +10,11 @@
 
 namespace remoting {
 
-// TODO(ajwong): Once ChromotingPlugin stablizes a little more, come up with
-// sane unittests.
+// TODO(ajwong): Fill this out.
 class ClientUtilTest : public testing::Test {
  protected:
   virtual void SetUp() {
   }
 };
 
-TEST_F(ClientUtilTest, GetLoginInfoFromUrlParams) {
-  const char url[] = "chromotocol://hostid?user=auser&auth=someauth&jid=ajid";
-  std::string user_id;
-  std::string auth_token;
-  std::string host_jid;
-  ClientConfig config;
-
-  ASSERT_TRUE(GetLoginInfoFromUrlParams(url, &config));
-
-  EXPECT_EQ("auser", config.username);
-  EXPECT_EQ("someauth", config.auth_token);
-  EXPECT_EQ("ajid", config.host_jid);
-}
-
 }  // namespace remoting
diff --git a/remoting/client/plugin/chromoting_plugin.cc b/remoting/client/plugin/chromoting_plugin.cc
index af7321e..3814be5 100644
--- a/remoting/client/plugin/chromoting_plugin.cc
+++ b/remoting/client/plugin/chromoting_plugin.cc
@@ -15,12 +15,13 @@
 #include "remoting/client/chromoting_client.h"
 #include "remoting/client/host_connection.h"
 #include "remoting/client/jingle_host_connection.h"
+#include "remoting/client/plugin/chromoting_scriptable_object.h"
 #include "remoting/client/plugin/pepper_input_handler.h"
 #include "remoting/client/plugin/pepper_view.h"
 #include "remoting/jingle_glue/jingle_thread.h"
 #include "third_party/ppapi/c/pp_event.h"
-#include "third_party/ppapi/c/pp_rect.h"
 #include "third_party/ppapi/cpp/completion_callback.h"
+#include "third_party/ppapi/cpp/rect.h"
 
 namespace remoting {
 
@@ -40,9 +41,7 @@ ChromotingPlugin::~ChromotingPlugin() {
   // to the message loop before this point.  Right now, we don't have a well
   // defined stop for the plugin process, and the thread shutdown is likely a
   // race condition.
-  if (context_.get()) {
-    context_->Stop();
-  }
+  context_.Stop();
 }
 
 bool ChromotingPlugin::Init(uint32_t argc,
@@ -63,60 +62,48 @@ bool ChromotingPlugin::Init(uint32_t argc,
   pepper_main_loop_dont_post_to_me_ = MessageLoop::current();
   LOG(INFO) << "Started ChromotingPlugin::Init";
 
-  // Extract the URL from the arguments.
-  const char* url = NULL;
-  for (uint32_t i = 0; i < argc; ++i) {
-    if (strcmp(argn[i], "src") == 0) {
-      url = argv[i];
-      break;
-    }
-  }
-
-  if (!url) {
-    return false;
-  }
-
-  ClientConfig config;
-  if (!GetLoginInfoFromUrlParams(url, &config)) {
-    LOG(WARNING) << "Could not parse URL: " << url;
-    return false;
-  }
+  // Start all the threads.
+  context_.Start();
 
   // Create the chromoting objects.
-  host_connection_.reset(new JingleHostConnection(context_.get()));
+  host_connection_.reset(new JingleHostConnection(&context_));
   view_.reset(new PepperView(this));
   input_handler_.reset(new PepperInputHandler());
+
+  // Default to a medium grey.
+  view_->SetSolidFill(0xFFCDCDCD);
+
+  return true;
+}
+
+void ChromotingPlugin::Connect(const ClientConfig& config) {
+  DCHECK(CurrentlyOnPluginThread());
+
   client_.reset(new ChromotingClient(config,
-                                     context_.get(),
+                                     &context_,
                                      host_connection_.get(),
                                      view_.get(),
                                      input_handler_.get(),
                                      NULL));
 
-  // Default to a medium grey.
-  view_->SetSolidFill(0xFFCDCDCD);
-
   // Kick off the connection.
-  context_->Start();
   client_->Start();
-
-  return true;
 }
 
-void ChromotingPlugin::ViewChanged(const PP_Rect& position,
-                                   const PP_Rect& clip) {
+void ChromotingPlugin::ViewChanged(const pp::Rect& position,
+                                   const pp::Rect& clip) {
   DCHECK(CurrentlyOnPluginThread());
 
   // TODO(ajwong): This is going to be a race condition when the view changes
   // and we're in the middle of a Paint().
   LOG(INFO) << "ViewChanged "
-            << position.point.x << ","
-            << position.point.y << ","
-            << position.size.width << ","
-            << position.size.height;
+            << position.x() << ","
+            << position.y() << ","
+            << position.width() << ","
+            << position.height();
 
-  view_->SetViewport(position.point.x, position.point.y,
-                       position.size.width, position.size.height);
+  view_->SetViewport(position.x(), position.y(),
+                     position.width(), position.height());
   view_->Paint();
 }
 
@@ -147,4 +134,18 @@ bool ChromotingPlugin::HandleEvent(const PP_Event& event) {
   return false;
 }
 
+pp::Var ChromotingPlugin::GetInstanceObject() {
+  LOG(ERROR) << "Getting instance object.";
+  if (instance_object_.is_void()) {
+    ChromotingScriptableObject* object = new ChromotingScriptableObject(this);
+    object->Init();
+
+    LOG(ERROR) << "Object initted.";
+    // The pp::Var takes ownership of object here.
+    instance_object_ = pp::Var(object);
+  }
+
+  return instance_object_;
+}
+
 }  // namespace remoting
diff --git a/remoting/client/plugin/chromoting_plugin.h b/remoting/client/plugin/chromoting_plugin.h
index 3bd0470..d613a97 100644
--- a/remoting/client/plugin/chromoting_plugin.h
+++ b/remoting/client/plugin/chromoting_plugin.h
@@ -12,6 +12,7 @@
 
 #include "base/at_exit.h"
 #include "base/scoped_ptr.h"
+#include "remoting/client/client_context.h"
 #include "remoting/client/host_connection.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"
 #include "third_party/ppapi/c/pp_event.h"
@@ -20,6 +21,7 @@
 #include "third_party/ppapi/c/pp_resource.h"
 #include "third_party/ppapi/cpp/instance.h"
 #include "third_party/ppapi/cpp/device_context_2d.h"
+#include "third_party/ppapi/cpp/var.h"
 
 class MessageLoop;
 
@@ -49,13 +51,14 @@ class ChromotingPlugin : public pp::Instance {
   virtual ~ChromotingPlugin();
 
   virtual bool Init(uint32_t argc, const char* argn[], const char* argv[]);
+  virtual void Connect(const ClientConfig& config);
   virtual bool HandleEvent(const PP_Event& event);
-  virtual void ViewChanged(const PP_Rect& position, const PP_Rect& clip);
+  virtual pp::Var GetInstanceObject();
+  virtual void ViewChanged(const pp::Rect& position, const pp::Rect& clip);
 
   virtual bool CurrentlyOnPluginThread() const;
 
  private:
-  FRIEND_TEST(ChromotingPluginTest, ParseUrl);
   FRIEND_TEST(ChromotingPluginTest, TestCaseSetup);
 
   // Since we're an internal plugin, we can just grab the message loop during
@@ -66,15 +69,12 @@ class ChromotingPlugin : public pp::Instance {
   // TODO(ajwong): Think if there is a better way to safeguard this.
   MessageLoop* pepper_main_loop_dont_post_to_me_;
 
-  scoped_ptr<ClientContext> context_;
-
+  ClientContext context_;
   scoped_ptr<HostConnection> host_connection_;
-
   scoped_ptr<PepperView> view_;
-
   scoped_ptr<InputHandler> input_handler_;
-
   scoped_ptr<ChromotingClient> client_;
+  pp::Var instance_object_;  // JavaScript interface to control this instance.
 
   DISALLOW_COPY_AND_ASSIGN(ChromotingPlugin);
 };
diff --git a/remoting/client/plugin/chromoting_scriptable_object.cc b/remoting/client/plugin/chromoting_scriptable_object.cc
new file mode 100644
index 0000000..6c26e38
--- /dev/null
+++ b/remoting/client/plugin/chromoting_scriptable_object.cc
@@ -0,0 +1,177 @@
+// Copyright 2010 Google Inc. All Rights Reserved.
+// Author: ajwong@google.com (Albert Wong)
+
+#include "remoting/client/plugin/chromoting_scriptable_object.h"
+
+#include "remoting/client/client_config.h"
+#include "remoting/client/plugin/chromoting_plugin.h"
+
+#include "third_party/ppapi/cpp/var.h"
+
+using pp::Var;
+
+namespace remoting {
+ChromotingScriptableObject::ChromotingScriptableObject(
+    ChromotingPlugin* instance)
+    : instance_(instance) {
+}
+
+ChromotingScriptableObject::~ChromotingScriptableObject() {
+}
+
+void ChromotingScriptableObject::Init() {
+  // Property addition order should match the interface description at the
+  // top of chromoting_scriptable_object.h.
+  AddAttribute("onreadystatechange", Var());
+
+  AddAttribute("NOT_CONNECTED", Var(0));
+  AddAttribute("CONNECTED", Var(1));
+
+  AddMethod("connect", &ChromotingScriptableObject::DoConnect);
+
+  // TODO(ajwong): Figure out how to implement the status variable.
+  AddAttribute("status", Var("not_implemented_yet"));
+}
+
+bool ChromotingScriptableObject::HasProperty(const Var& name, Var* exception) {
+  // TODO(ajwong): Check if all these name.is_string() sentinels are required.
+  if (!name.is_string()) {
+    *exception = Var("HasProperty expects a string for the name.");
+    return false;
+  }
+
+  PropertyNameMap::const_iterator iter = property_names_.find(name.AsString());
+  if (iter == property_names_.end()) {
+    return false;
+  }
+
+  // TODO(ajwong): Investigate why ARM build breaks if you do:
+  //    properties_[iter->second].method == NULL;
+  // Somehow the ARM compiler is thinking that the above is using
+  // NULL as an arithmetic expression.
+  return properties_[iter->second].method == 0;
+}
+
+bool ChromotingScriptableObject::HasMethod(const Var& name, Var* exception) {
+  // TODO(ajwong): Check if all these name.is_string() sentinels are required.
+  if (!name.is_string()) {
+    *exception = Var("HasMethod expects a string for the name.");
+    return false;
+  }
+
+  PropertyNameMap::const_iterator iter = property_names_.find(name.AsString());
+  if (iter == property_names_.end()) {
+    return false;
+  }
+
+  // See comment from HasProperty about why to use 0 instead of NULL here.
+  return properties_[iter->second].method != 0;
+}
+
+Var ChromotingScriptableObject::GetProperty(const Var& name, Var* exception) {
+  // TODO(ajwong): Check if all these name.is_string() sentinels are required.
+  if (!name.is_string()) {
+    *exception = Var("GetProperty expects a string for the name.");
+    return Var();
+  }
+
+  PropertyNameMap::const_iterator iter = property_names_.find(name.AsString());
+
+  // No property found.
+  if (iter == property_names_.end()) {
+    return ScriptableObject::GetProperty(name, exception);
+  }
+
+  // TODO(ajwong): This incorrectly return a null object if a function
+  // property is requested.
+  return properties_[iter->second].attribute;
+}
+
+void ChromotingScriptableObject::GetAllPropertyNames(
+    std::vector<Var>* properties,
+    Var* exception) {
+  for (size_t i = 0; i < properties_.size(); i++) {
+    properties->push_back(Var(properties_[i].name));
+  }
+}
+
+void ChromotingScriptableObject::SetProperty(const Var& name,
+                                             const Var& value,
+                                             Var* exception) {
+  // TODO(ajwong): Check if all these name.is_string() sentinels are required.
+  if (!name.is_string()) {
+    *exception = Var("SetProperty expects a string for the name.");
+    return;
+  }
+
+  // Only allow writing to onreadystatechange.  See top of
+  // chromoting_scriptable_object.h for the object interface definition.
+  std::string property_name = name.AsString();
+  if (property_name != "onstatechange") {
+    *exception =
+        Var("Cannot set property " + property_name + " on this object.");
+    return;
+  }
+
+  // Since we're whitelisting the propertie that are settable above, we can
+  // assume that the property exists in the map.
+  properties_[property_names_[property_name]].attribute = value;
+}
+
+Var ChromotingScriptableObject::Call(const Var& method_name,
+                                     const std::vector<Var>& args,
+                                     Var* exception) {
+  PropertyNameMap::const_iterator iter =
+      property_names_.find(method_name.AsString());
+  if (iter == property_names_.end()) {
+    return pp::ScriptableObject::Call(method_name, args, exception);
+  }
+
+  return (this->*(properties_[iter->second].method))(args, exception);
+}
+
+void ChromotingScriptableObject::AddAttribute(const std::string& name,
+                                              Var attribute) {
+  property_names_[name] = properties_.size();
+  properties_.push_back(PropertyDescriptor(name, attribute));
+}
+
+void ChromotingScriptableObject::AddMethod(const std::string& name,
+                                           MethodHandler handler) {
+  property_names_[name] = properties_.size();
+  properties_.push_back(PropertyDescriptor(name, handler));
+}
+
+pp::Var ChromotingScriptableObject::DoConnect(const std::vector<Var>& args,
+                                           Var* exception) {
+  if (args.size() != 3) {
+    *exception = Var("Usage: connect(username, host_jid, auth_token");
+    return Var();
+  }
+
+  ClientConfig config;
+
+  if (!args[0].is_string()) {
+    *exception = Var("The username must be a string.");
+    return Var();
+  }
+  config.username = args[0].AsString();
+
+  if (!args[1].is_string()) {
+    *exception = Var("The host_jid must be a string.");
+    return Var();
+  }
+  config.host_jid = args[1].AsString();
+
+  if (!args[2].is_string()) {
+    *exception = Var("The auth_token must be a string.");
+    return Var();
+  }
+  config.auth_token = args[2].AsString();
+
+  instance_->Connect(config);
+
+  return Var();
+}
+
+}  // namespace remoting
diff --git a/remoting/client/plugin/chromoting_scriptable_object.h b/remoting/client/plugin/chromoting_scriptable_object.h
new file mode 100644
index 0000000..3513120
--- /dev/null
+++ b/remoting/client/plugin/chromoting_scriptable_object.h
@@ -0,0 +1,100 @@
+// Copyright (c) 2010 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.
+
+// This implements the JavaScript class entrypoint for the plugin.
+// The Javascript API is defined as follows.
+//
+// interface ChromotingScriptableObject {
+//   // Called when the Chromoting plugin has had a state change such as
+//   // connection completed.
+//   attribute Function onreadystatechange;
+//
+//   // Constants for states, etc.
+//   const unsigned short NOT_CONNECTED = 0;
+//   const unsigned short CONNECTED = 1;
+//
+//   // Methods on the object.
+//   void connect(string username, string host_jid, string auth_token);
+//
+//   // Attributes.
+//   readonly attribute unsigned short status;
+// }
+//
+//  onreadystatechange
+//
+// Methods:
+//   Connect(username, auth_token, host_jid, onstatechange);
+
+#ifndef REMOTING_CLIENT_PLUGIN_CHROMOTING_SCRIPTABLE_OBJECT_H_
+#define REMOTING_CLIENT_PLUGIN_CHROMOTING_SCRIPTABLE_OBJECT_H_
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "third_party/ppapi/cpp/scriptable_object.h"
+#include "third_party/ppapi/cpp/var.h"
+
+namespace remoting {
+
+class ChromotingPlugin;
+
+class ChromotingScriptableObject : public pp::ScriptableObject {
+ public:
+  explicit ChromotingScriptableObject(ChromotingPlugin* instance);
+  virtual ~ChromotingScriptableObject();
+
+  virtual void Init();
+
+  // Override the ScriptableObject functions.
+  virtual bool HasProperty(const pp::Var& name, pp::Var* exception);
+  virtual bool HasMethod(const pp::Var& name, pp::Var* exception);
+  virtual pp::Var GetProperty(const pp::Var& name, pp::Var* exception);
+  virtual void GetAllPropertyNames(std::vector<pp::Var>* properties,
+                                   pp::Var* exception);
+  virtual void SetProperty(const pp::Var& name,
+                           const pp::Var& value,
+                           pp::Var* exception);
+  virtual pp::Var Call(const pp::Var& method_name,
+                       const std::vector<pp::Var>& args,
+                       pp::Var* exception);
+
+ private:
+  typedef std::map<std::string, int> PropertyNameMap;
+  typedef pp::Var (ChromotingScriptableObject::*MethodHandler)(
+      const std::vector<pp::Var>& args, pp::Var* exception);
+  struct PropertyDescriptor {
+    explicit PropertyDescriptor(const std::string& n, pp::Var a)
+        : name(n), attribute(a), method(NULL) {
+    }
+
+    explicit PropertyDescriptor(const std::string& n, MethodHandler m)
+        : name(n), method(m) {
+    }
+
+    enum Type {
+      ATTRIBUTE,
+      METHOD,
+    } type;
+
+    std::string name;
+    pp::Var attribute;
+    MethodHandler method;
+  };
+
+
+  void AddAttribute(const std::string& name, pp::Var attribute);
+  void AddMethod(const std::string& name, MethodHandler handler);
+
+  pp::Var DoConnect(const std::vector<pp::Var>& args, pp::Var* exception);
+
+  PropertyNameMap property_names_;
+  std::vector<PropertyDescriptor> properties_;
+
+  ChromotingPlugin* instance_;
+};
+
+}  // namespace remoting
+
+#endif  // REMOTING_CLIENT_PLUGIN_CHROMOTING_SCRIPTABLE_OBJECT_H_
diff --git a/remoting/client/plugin/pepper_view.cc b/remoting/client/plugin/pepper_view.cc
index c2f3538..9e6a7af 100644
--- a/remoting/client/plugin/pepper_view.cc
+++ b/remoting/client/plugin/pepper_view.cc
@@ -43,12 +43,14 @@ void PepperView::Paint() {
     return;
   }
 
-  // TODO(ajwong): We shouldn't assume the image data format.
-  pp::ImageData image(PP_IMAGEDATAFORMAT_BGRA_PREMUL,
+  // TODO(ajwong): We're assuming the native format is BGRA_PREMUL below. This
+  // is wrong.
+  pp::ImageData image(pp::ImageData::GetNativeImageDataFormat(),
                       pp::Size(viewport_width_, viewport_height_),
                       false);
   if (image.is_null()) {
-    LOG(ERROR) << "Unable to allocate image.";
+    LOG(ERROR) << "Unable to allocate image of size: "
+               << viewport_width_ << "x" << viewport_height_;
     return;
   }
 
diff --git a/remoting/client/x11_view.cc b/remoting/client/x11_view.cc
index fe30291..ef3116b 100644
--- a/remoting/client/x11_view.cc
+++ b/remoting/client/x11_view.cc
@@ -73,7 +73,7 @@ void X11View::TearDown() {
 
 void X11View::Paint() {
   // Don't bother attempting to paint if the display hasn't been set up.
-  if (!display_ || !window_ || !height_ || !width_) {
+  if (!display_ || !window_ || !height_ || !width_ || !frame_) {
     return;
   }
 
@@ -146,6 +146,8 @@ void X11View::SetHostScreenSize(int width, int height) {
   width_ = width;
   height_ = height;
   XResizeWindow(display_, window_, width_, height_);
+  media::VideoFrame::CreateFrame(media::VideoFrame::RGB32, width_, height_,
+                                 base::TimeDelta(), base::TimeDelta(), &frame_);
 }
 
 void X11View::InitPaintTarget() {
diff --git a/remoting/remoting.gyp b/remoting/remoting.gyp
index 10dccf7..8b2dec6 100644
--- a/remoting/remoting.gyp
+++ b/remoting/remoting.gyp
@@ -78,6 +78,8 @@
       'sources': [
         'client/plugin/chromoting_plugin.cc',
         'client/plugin/chromoting_plugin.h',
+        'client/plugin/chromoting_scriptable_object.cc',
+        'client/plugin/chromoting_scriptable_object.h',
         'client/plugin/pepper_entrypoints.cc',
         'client/plugin/pepper_entrypoints.h',
         'client/plugin/pepper_input_handler.cc',