Code review changes. Incorporated all the suggestions from previous review.

One item not changed yet is to rename 'receiver' in ForwardMessageToExternalHost.
The idea is to invoke receiver("message") at the other end. So for example if the
args to ForwardMessageToExternalHost("hello", "world") then we will invoke a
script hello("world") on the other side.

'receiver' doesn't really describe the first argument here so if there is a 
better suggestion, I would be happy to change it :)


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

4 files changed, 44 insertions, 69 deletions

diff --git a/chrome/renderer/external_host_bindings.cc b/chrome/renderer/external_host_bindings.cc
index ffcd5fd..9ff8a54 100644
--- a/chrome/renderer/external_host_bindings.cc
+++ b/chrome/renderer/external_host_bindings.cc
@@ -29,22 +29,16 @@
 
 #include "chrome/renderer/external_host_bindings.h"
 
-#include "base/json_writer.h"
-#include "base/scoped_handle.h"
 #include "base/values.h"
 #include "chrome/common/render_messages.h"
-#include "chrome/common/stl_util-inl.h"
 
-ExternalHostBindings::ExternalHostBindings() : routing_id_(0), sender_(NULL) {
-  BindMethod("postMessage", &ExternalHostBindings::postMessage);
+void ExternalHostBindings::BindMethods() {
+  BindMethod("ForwardMessageToExternalHost",
+             &ExternalHostBindings::ForwardMessageToExternalHost);
 }
 
-ExternalHostBindings::~ExternalHostBindings() {
-  STLDeleteContainerPointers(properties_.begin(), properties_.end());
-}
-
-void ExternalHostBindings::postMessage(const CppArgumentList& args,
-                                       CppVariant* result) {
+void ExternalHostBindings::ForwardMessageToExternalHost(
+    const CppArgumentList& args, CppVariant* result) {
   // We expect at least a string message identifier, and optionally take
   // an object parameter.  If we get anything else we bail.
   if (args.size() < 2)
@@ -54,17 +48,9 @@ void ExternalHostBindings::postMessage(const CppArgumentList& args,
   if (!args[0].isString() && !args[1].isString())
     return;
 
-  const std::string receiver = args[0].ToString();
-  const std::string message = args[1].ToString();
-
-  sender_->Send(
-      new ViewHostMsg_ExternalHostMessage(routing_id_, receiver, message));
-}
+  const std::string& receiver = args[0].ToString();
+  const std::string& message = args[1].ToString();
 
-void ExternalHostBindings::SetProperty(const std::string& name,
-                                       const std::string& value) {
-  CppVariant* cpp_value = new CppVariant;
-  cpp_value->Set(value);
-  BindProperty(name, cpp_value);
-  properties_.push_back(cpp_value);
+  sender()->Send(new ViewHostMsg_ForwardMessageToExternalHost(
+      routing_id(), receiver, message));
 }
diff --git a/chrome/renderer/external_host_bindings.h b/chrome/renderer/external_host_bindings.h
index dde86af..3c56ee1 100644
--- a/chrome/renderer/external_host_bindings.h
+++ b/chrome/renderer/external_host_bindings.h
@@ -27,48 +27,30 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-#ifndef CHROME_RENDERER_EXTERNAL_HOST_BINDINGS_H__
-#define CHROME_RENDERER_EXTERNAL_HOST_BINDINGS_H__
+#ifndef CHROME_RENDERER_EXTERNAL_HOST_BINDINGS_H_
+#define CHROME_RENDERER_EXTERNAL_HOST_BINDINGS_H_
 
 #include "chrome/common/ipc_message.h"
-#include "webkit/glue/cpp_bound_class.h"
+#include "dom_ui_bindings.h"
 
 // ExternalHostBindings is the class backing the "externalHost" object
 // accessible from Javascript
 //
 // We expose one function, for sending a message to the external host:
-//  postMessage(String receiver, String message);
-class ExternalHostBindings : public CppBoundClass {
+//  ForwardMessageToExternalHost(String receiver, String message);
+class ExternalHostBindings : public DOMBoundBrowserObject {
  public:
-  ExternalHostBindings();
-  ~ExternalHostBindings();
+  ExternalHostBindings() { BindMethods(); }
+  virtual ~ExternalHostBindings() {};
 
-  // The postMessage() function provided to Javascript.
-  void postMessage(const CppArgumentList& args, CppVariant* result);
-
-  // Set the message channel back to the browser.
-  void set_message_sender(IPC::Message::Sender* sender) {
-    sender_ = sender;
-  }
-
-  // Set the routing id for messages back to the browser.
-  void set_routing_id(int routing_id) {
-    routing_id_ = routing_id;
-  }
-
-  // Sets a property with the given name and value.
-  void SetProperty(const std::string& name, const std::string& value);
+  // DOMBoundBrowserObject implementation.
+  virtual void BindMethods();
 
+  // The ForwardMessageToExternalHost() function provided to Javascript.
+  void ForwardMessageToExternalHost(const CppArgumentList& args,
+                                    CppVariant* result);
  private:
-  // Our channel back to the browser is a message sender
-  // and routing id.
-  IPC::Message::Sender* sender_;
-  int routing_id_;
-
-  // The list of properties that have been set.  We keep track of this so we
-  // can free them on destruction.
-  typedef std::vector<CppVariant*> PropertyList;
-  PropertyList properties_;
+  DISALLOW_COPY_AND_ASSIGN(ExternalHostBindings);
 };
 
-#endif  // CHROME_RENDERER_DOM_UI_BINDINGS_H__
+#endif  // CHROME_RENDERER_EXTERNAL_HOST_BINDINGS_H_
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index 41b6ab0..d7ec667 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -335,7 +335,7 @@ void RenderView::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_HANDLER(ViewMsg_DragTargetDrop, OnDragTargetDrop)
     IPC_MESSAGE_HANDLER(ViewMsg_AllowDomAutomationBindings,
                         OnAllowDomAutomationBindings)
-    IPC_MESSAGE_HANDLER(ViewMsg_AllowDOMUIBindings, OnAllowDOMUIBindings)
+    IPC_MESSAGE_HANDLER(ViewMsg_AllowBindings, OnAllowBindings)
     IPC_MESSAGE_HANDLER(ViewMsg_SetDOMUIProperty, OnSetDOMUIProperty)
     IPC_MESSAGE_HANDLER(ViewMsg_DragSourceEndedOrMoved, OnDragSourceEndedOrMoved)
     IPC_MESSAGE_HANDLER(ViewMsg_DragSourceSystemDragEnded,
@@ -358,7 +358,8 @@ void RenderView::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_HANDLER(ViewMsg_ShouldClose, OnMsgShouldClose)
     IPC_MESSAGE_HANDLER(ViewMsg_ClosePage, OnClosePage)
     IPC_MESSAGE_HANDLER(ViewMsg_ThemeChanged, OnThemeChanged)
-    IPC_MESSAGE_HANDLER(ViewMsg_PostMessage, OnPostMessage)
+    IPC_MESSAGE_HANDLER(ViewMsg_HandleMessageFromExternalHost,
+                        OnMessageFromExternalHost)
     // Have the super handle all other messages.
     IPC_MESSAGE_UNHANDLED(RenderWidget::OnMessageReceived(message))
   IPC_END_MESSAGE_MAP()
@@ -1418,10 +1419,11 @@ void RenderView::WindowObjectCleared(WebFrame* webframe) {
     dom_ui_bindings_.set_routing_id(routing_id_);
     dom_ui_bindings_.BindToJavascript(webframe, L"chrome");
   }
-
-  external_host_bindings_.set_message_sender(this);
-  external_host_bindings_.set_routing_id(routing_id_);
-  external_host_bindings_.BindToJavascript(webframe, L"externalHost");
+  if (enable_external_host_bindings_) {
+    external_host_bindings_.set_message_sender(this);
+    external_host_bindings_.set_routing_id(routing_id_);
+    external_host_bindings_.BindToJavascript(webframe, L"externalHost");
+  }
 
 #ifdef CHROME_PERSONALIZATION
   Personalization::ConfigureRendererPersonalization(personalization_, this,
@@ -2281,8 +2283,11 @@ void RenderView::OnAllowDomAutomationBindings(bool allow_bindings) {
   enable_dom_automation_ = allow_bindings;
 }
 
-void RenderView::OnAllowDOMUIBindings() {
-  enable_dom_ui_bindings_ = true;
+void RenderView::OnAllowBindings(bool enable_dom_ui_bindings,
+                                 bool enable_external_host_bindings)
+{
+  enable_dom_ui_bindings_ = enable_dom_ui_bindings;
+  enable_external_host_bindings_ = enable_external_host_bindings;
 }
 
 void RenderView::OnSetDOMUIProperty(const std::string& name,
@@ -2500,8 +2505,8 @@ void RenderView::OnThemeChanged() {
   DidInvalidateRect(webwidget_, view_rect);
 }
 
-void RenderView::OnPostMessage(const std::string& target,
-                               const std::string& message) {
+void RenderView::OnMessageFromExternalHost(
+    const std::string& target, const std::string& message) {
   if (message.empty())
     return;
 
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index cecebaa..7d51aa0 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -416,7 +416,8 @@ class RenderView : public RenderWidget, public WebViewDelegate,
   void OnDragTargetDrop(const gfx::Point& client_pt,
                         const gfx::Point& screen_pt);
   void OnAllowDomAutomationBindings(bool allow_binding);
-  void OnAllowDOMUIBindings();
+  void OnAllowBindings(bool enable_dom_ui_bindings,
+                       bool enable_external_host_bindings);
   void OnSetDOMUIProperty(const std::string& name, const std::string& value);
   void OnSetInitialFocus(bool reverse);
   void OnUpdateWebPreferences(const WebPreferences& prefs);
@@ -455,9 +456,9 @@ class RenderView : public RenderWidget, public WebViewDelegate,
   // Notification about ui theme changes.
   void OnThemeChanged();
 
-  // Handles messages posted by the browser.
-  void OnPostMessage(const std::string& target,
-                     const std::string& message);
+  // Handles messages posted from automation.
+  void OnMessageFromExternalHost(const std::string& target,
+                                 const std::string& message);
 
   // Switches the frame's CSS media type to "print" and calculate the number of
   // printed pages that are to be expected. |frame| will be used to calculate
@@ -521,6 +522,7 @@ class RenderView : public RenderWidget, public WebViewDelegate,
   ExternalJSObject external_js_object_;
 
   // External host exposed through automation controller.
+  bool enable_external_host_bindings_;
   ExternalHostBindings external_host_bindings_;
 
   // The last gotten main frame's encoding.