Remove wstring from RVH's run Javascript command....

Remove wstring from RVH's run Javascript command.

BUG=23581
TEST=no visible changes; all tests pass


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

[Avi (use Gerrit), 2011-02-04 20:29:33]

Nirnimesh: pyauto stuff
Evan: all else

Yeah, sorry, this got out of control.

[Evan Martin, 2011-02-04 20:31:51]

65 files ಠ_ಠ

[Evan Martin, 2011-02-04 21:37:29]

http://codereview.chromium.org/6312154/diff/1/chrome/browser/automation/autom...
File chrome/browser/automation/automation_provider_observers.h (right):

http://codereview.chromium.org/6312154/diff/1/chrome/browser/automation/autom...
chrome/browser/automation/automation_provider_observers.h:911: void
ExecuteScript(const std::string& javascript);
In theory this can be Unicode.  (In particular, consider passing a JSON object
down.)  It looks like you're using UTF-8 elsewhere.  Should this be a string16
instead?

http://codereview.chromium.org/6312154/diff/1/chrome/browser/automation/testi...
File chrome/browser/automation/testing_automation_provider.h (right):

http://codereview.chromium.org/6312154/diff/1/chrome/browser/automation/testi...
chrome/browser/automation/testing_automation_provider.h:273: int64 parent_id,
int index, string16 title,
const string16&

http://codereview.chromium.org/6312154/diff/1/chrome/browser/automation/testi...
chrome/browser/automation/testing_automation_provider.h:277: string16 title,
const GURL& url,
const string16&

http://codereview.chromium.org/6312154/diff/1/chrome/browser/automation/testi...
chrome/browser/automation/testing_automation_provider.h:283: int64 id, string16
title,
const string16&

http://codereview.chromium.org/6312154/diff/1/chrome/browser/browser_browsert...
File chrome/browser/browser_browsertest.cc (right):

http://codereview.chromium.org/6312154/diff/1/chrome/browser/browser_browsert...
chrome/browser/browser_browsertest.cc:55: const wchar_t*
OPEN_NEW_BEFOREUNLOAD_PAGE =
char*

Can you fix these to kOpenNewBeforeUnloadPage (and same for L50?)

http://codereview.chromium.org/6312154/diff/1/chrome/browser/browser_browsert...
chrome/browser/browser_browsertest.cc:280:
WideToUTF16(OPEN_NEW_BEFOREUNLOAD_PAGE));
ASCIIToUTF16 once char* change

http://codereview.chromium.org/6312154/diff/1/chrome/browser/debugger/devtool...
File chrome/browser/debugger/devtools_window.cc (right):

http://codereview.chromium.org/6312154/diff/1/chrome/browser/debugger/devtool...
chrome/browser/debugger/devtools_window.cc:316: string16 javascript =
function_name + char16('(') + UTF8ToUTF16(json) +
Is char16() needed here?  I think '(' should be sufficient?

http://codereview.chromium.org/6312154/diff/1/chrome/browser/ui/tests/browser...
File chrome/browser/ui/tests/browser_uitest.cc (right):

http://codereview.chromium.org/6312154/diff/1/chrome/browser/ui/tests/browser...
chrome/browser/ui/tests/browser_uitest.cc:289: tmp_profile_ =
tmp_profile_.Append(L"Test Chrome G\u00E9raldine");
Good fix!

[Nirnimesh, 2011-02-04 21:59:18]

For Pyauto, if it compiles it's fine.

http://codereview.chromium.org/6312154/diff/1/chrome/browser/automation/autom...
File chrome/browser/automation/automation_provider_observers.h (right):

http://codereview.chromium.org/6312154/diff/1/chrome/browser/automation/autom...
chrome/browser/automation/automation_provider_observers.h:911: void
ExecuteScript(const std::string& javascript);
On 2011/02/04 21:37:29, Evan Martin wrote:
> In theory this can be Unicode.  (In particular, consider passing a JSON object
> down.)  It looks like you're using UTF-8 elsewhere.  Should this be a string16
> instead?

+1. We have tests that might pass non-ascii

http://codereview.chromium.org/6312154/diff/1/chrome/test/pyautolib/pyautolib.cc
File chrome/test/pyautolib/pyautolib.cc (right):

http://codereview.chromium.org/6312154/diff/1/chrome/test/pyautolib/pyautolib...
chrome/test/pyautolib/pyautolib.cc:244: bool
PyUITestBase::AddBookmarkGroup(const std::wstring& parent_id, int index,
string16 here?

[Avi (use Gerrit), 2011-02-04 23:02:10]

"if it compiles it's fine" is awesome, but don't I have to worry about the
callers of those pyauto functions?

http://codereview.chromium.org/6312154/diff/1/chrome/browser/automation/autom...
File chrome/browser/automation/automation_provider_observers.h (right):

http://codereview.chromium.org/6312154/diff/1/chrome/browser/automation/autom...
chrome/browser/automation/automation_provider_observers.h:911: void
ExecuteScript(const std::string& javascript);
OK

http://codereview.chromium.org/6312154/diff/1/chrome/browser/automation/testi...
File chrome/browser/automation/testing_automation_provider.h (right):

http://codereview.chromium.org/6312154/diff/1/chrome/browser/automation/testi...
chrome/browser/automation/testing_automation_provider.h:273: int64 parent_id,
int index, string16 title,
On 2011/02/04 21:37:29, Evan Martin wrote:
> const string16&

Done.

http://codereview.chromium.org/6312154/diff/1/chrome/browser/automation/testi...
chrome/browser/automation/testing_automation_provider.h:277: string16 title,
const GURL& url,
On 2011/02/04 21:37:29, Evan Martin wrote:
> const string16&

Done.

http://codereview.chromium.org/6312154/diff/1/chrome/browser/automation/testi...
chrome/browser/automation/testing_automation_provider.h:283: int64 id, string16
title,
On 2011/02/04 21:37:29, Evan Martin wrote:
> const string16&

Done.

http://codereview.chromium.org/6312154/diff/1/chrome/browser/browser_browsert...
File chrome/browser/browser_browsertest.cc (right):

http://codereview.chromium.org/6312154/diff/1/chrome/browser/browser_browsert...
chrome/browser/browser_browsertest.cc:55: const wchar_t*
OPEN_NEW_BEFOREUNLOAD_PAGE =
On 2011/02/04 21:37:29, Evan Martin wrote:
> char*
> 
> Can you fix these to kOpenNewBeforeUnloadPage (and same for L50?)

Done.

http://codereview.chromium.org/6312154/diff/1/chrome/browser/browser_browsert...
chrome/browser/browser_browsertest.cc:280:
WideToUTF16(OPEN_NEW_BEFOREUNLOAD_PAGE));
On 2011/02/04 21:37:29, Evan Martin wrote:
> ASCIIToUTF16 once char* change

Done.

http://codereview.chromium.org/6312154/diff/1/chrome/browser/debugger/devtool...
File chrome/browser/debugger/devtools_window.cc (right):

http://codereview.chromium.org/6312154/diff/1/chrome/browser/debugger/devtool...
chrome/browser/debugger/devtools_window.cc:316: string16 javascript =
function_name + char16('(') + UTF8ToUTF16(json) +
On 2011/02/04 21:37:29, Evan Martin wrote:
> Is char16() needed here?  I think '(' should be sufficient?

No; there is no operator+ for string16 and char.

http://codereview.chromium.org/6312154/diff/1/chrome/browser/ui/tests/browser...
File chrome/browser/ui/tests/browser_uitest.cc (right):

http://codereview.chromium.org/6312154/diff/1/chrome/browser/ui/tests/browser...
chrome/browser/ui/tests/browser_uitest.cc:289: tmp_profile_ =
tmp_profile_.Append(L"Test Chrome G\u00E9raldine");
Presubmit complained. Why does no one listen?

http://codereview.chromium.org/6312154/diff/1/chrome/test/pyautolib/pyautolib.cc
File chrome/test/pyautolib/pyautolib.cc (right):

http://codereview.chromium.org/6312154/diff/1/chrome/test/pyautolib/pyautolib...
chrome/test/pyautolib/pyautolib.cc:244: bool
PyUITestBase::AddBookmarkGroup(const std::wstring& parent_id, int index,
On 2011/02/04 21:59:19, Nirnimesh wrote:
> string16 here?

Done.

[Nirnimesh, 2011-02-04 23:23:33]

No you don't have to worry. Swig takes care of converting python unicode strings
to string16 and vice versa.

If there's a mismatch (as defined in chrome/test/pyautolib/pyautolib.i/h/cc),
the compiler will point it out.

[Avi (use Gerrit), 2011-02-07 22:45:52]

Cool.

Evan, take another look. The "difference from v1" links should still be useful.

[Evan Martin, 2011-02-07 22:52:29]

LGTM

http://codereview.chromium.org/6312154/diff/1076/chrome/test/webdriver/automa...
File chrome/test/webdriver/automation.cc (right):

http://codereview.chromium.org/6312154/diff/1076/chrome/test/webdriver/automa...
chrome/test/webdriver/automation.cc:57: xpath16, script16, &result16);
Maybe fits on one line?

[Avi (use Gerrit), 2011-02-07 22:55:25]

http://codereview.chromium.org/6312154/diff/1076/chrome/test/webdriver/automa...
File chrome/test/webdriver/automation.cc (right):

http://codereview.chromium.org/6312154/diff/1076/chrome/test/webdriver/automa...
chrome/test/webdriver/automation.cc:57: xpath16, script16, &result16);
On 2011/02/07 22:52:29, Evan Martin wrote:
> Maybe fits on one line?

Done.

[Avi (use Gerrit), 2011-02-08 19:52:43]

This is bad; apparently touching automation IPC kills bots. I don't know what
I'm doing. Reverting.

[Evan Martin, 2011-02-08 19:58:07]

On 2011/02/08 19:52:43, Avi wrote:
> This is bad; apparently touching automation IPC kills bots. I don't know what
> I'm doing. Reverting.

Oh yeah, I think you may need to update the reference build in parallel.

[Nirnimesh, 2011-02-08 22:28:13]

Avi, you might want to run pyauto tests before you commit again.
Build 'pyautolib' target
$ python chrome/test/functional/pyauto_functional.py