Adding the first set of remote test cases and associated framework.

chrome/test/remoting/remote_test_helper.h

+// Copyright 2014 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.
+
+#ifndef CHROME_TEST_REMOTING_REMOTE_TEST_HELPER_H_
+#define CHROME_TEST_REMOTING_REMOTE_TEST_HELPER_H_
+
+#include "content/public/test/browser_test_utils.h"
+
+namespace remoting {
+
+// Mirrored in host.js
+enum Action {
+  ERROR = -1,
+  NONE = 0,
+  KEYDOWN = 1,
+  BUTTONPRESS = 2,
+  MOUSEMOVE = 3,
+  MOUSEWHEEL = 4,
+  DRAG = 5,
+};
+
+typedef struct {
+  Action action = Action::NONE;

[Jamie, 2014/12/17 03:15:41]

I don't think this form of member initialization is permitted yet. See
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/zqB-DySA4V0
for details.

[Jamie, 2014/12/18 21:31:44]

You haven't addressed this comment.

[Mike Meade, 2015/01/05 23:18:58]

It looks like adding an inline constructor is what is being recommended now. I
have changed it to have a simple default constructor.

+  int value = 0;
+  int modifiers = 0;
+} Event;
+
+class RemoteTestHelper {
+ public:
+
+  // ctor.

[Jamie, 2014/12/17 03:15:41]

You don't need this comment.

[Mike Meade, 2014/12/18 18:54:24]

Done.

+  explicit RemoteTestHelper(content::WebContents* web_content)
+     :web_content_(web_content) {}

[Jamie, 2014/12/17 03:15:40]

Since you already have an .cc file for this class, I don't think you gain
anything by inlining this ctor.

[Mike Meade, 2014/12/18 18:54:24]

I got a lint warning indicating that since the only declared constructor takes a
parameter it needs to be declared explicit.

[Jamie, 2014/12/18 21:31:44]

The use of explicit is fine; it's the inlining of the ctor body that I don't
think is good style. In fact, if you had any non-trivial member variables, it
would be disallowed.

[Mike Meade, 2015/01/05 23:18:58]

Done.

+
+  // Execute an RPC call within the provided context.
+  void ExecuteRpc(const std::string& method, int timeout_secs = 10) {

[Jamie, 2014/12/17 03:15:40]

I don't think we're allowed to use default parameters. If we are, then it seems
odd to have a different default for timeout_secs for these two methods.

[Mike Meade, 2014/12/18 18:54:25]

Done.

+    ExecuteRpc(web_content_, method, timeout_secs);
+  }
+  static void ExecuteRpc(content::WebContents*,
+                         const std::string&,
+                         int timeout_secs = 2,
+                         int poll_ms = 250);
+
+  // Helper method to set the clear the last event
+  void ClearLastEvent() {
+    ClearLastEvent(web_content_);
+  }
+  static void ClearLastEvent(content::WebContents*);

[Jamie, 2014/12/17 03:15:40]

Do you need any of the methods that take an explicit WebContents? It seems like
we'd always want to use the implicit versions, and it's a shame to pollute the
interface with unnecessary additional methods.

[Mike Meade, 2014/12/18 18:54:25]

Done.

+
+  // Helper method to get the last event
+  void GetLastEvent(Event& event) {

[Jamie, 2014/12/17 03:15:40]

Please double-check, but I don't think the style guide allows non-const
references to be used as OUT parameters. Either make this function return an
Event (or a scoped_ptr to one) or pass the event by pointer.

[Mike Meade, 2014/12/18 18:54:25]

Done.

+      GetLastEvent(web_content_, event);
+    }

[Jamie, 2014/12/17 03:15:41]

Nit: indentation.

[Mike Meade, 2015/01/05 23:18:58]

Done.

+  static void GetLastEvent(content::WebContents*, Event&);
+
+ protected:

[Jamie, 2014/12/17 03:15:40]

It's not clear that this class is intended to be inherited from (no virtual
dtor, for example), so I'm not sure why you've separated out the protected vs.
private members.

[Mike Meade, 2014/12/18 18:54:24]

Done.

+  // Execute a script and extract the value as an integer
+  static int ExecuteScriptAndExtractInt(content::WebContents*,

[Jamie, 2014/12/17 03:15:40]

We already have definitions for these in RemoteDesktopBrowserTest. If we need
these versions as well, consider moving them out of the interface, since they're
not public.

[Mike Meade, 2014/12/18 18:54:25]

I moved the static ones from RemoteDesktopBrowserTest to here and changed the
references. The non-static versions are still in RemoteDesktopBrowserTest, they
just reference these methods now.

+                                        const std::string&);
+  // Execute a script and extract the value as a boolean
+  static bool ExecuteScriptAndExtractBool(content::WebContents*,
+                                        const std::string&);

[Jamie, 2014/12/17 03:15:41]

Nit: Indentation.

[Mike Meade, 2014/12/18 18:54:25]

Done.

+
+ private:

[Jamie, 2014/12/17 03:15:41]

You would typically have DISALLOW_COPY_AND_ASSIGN here (but you'll need to fix
up the places you're copying and assigning first).

[Mike Meade, 2014/12/18 18:54:24]

Done.

+  content::WebContents* web_content_;
+
+  // Check for a valid last event
+  static bool CheckForLastEvent(content::WebContents*);
+};
+
+}  // namespace remoting
+
+#endif  // CHROME_TEST_REMOTING_REMOTE_TEST_HELPER_H_