Translate: add RenderViewTest to check if translate.js handles errors

Translate: add RenderViewTest to check if translate.js handles errors

BUG=211308
TEST=browser_tests --gtest_filter='TranslateScriptBrowserTest.*'

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=222039

[Takashi Toyoshima, 2013-09-09 04:53:24]

This is missing tests for translate.js
See also https://chromiumcodereview.appspot.com/24024004/ you reviewed.

[hajimehoshi, 2013-09-09 06:17:41]

lgtm with nits

https://chromiumcodereview.appspot.com/23548019/diff/10001/chrome/renderer/tr...
File chrome/renderer/translate/translate_script_browsertest.cc (right):

https://chromiumcodereview.appspot.com/23548019/diff/10001/chrome/renderer/tr...
chrome/renderer/translate/translate_script_browsertest.cc:87: bool Error() {
GetError?

https://chromiumcodereview.appspot.com/23548019/diff/10001/chrome/renderer/tr...
chrome/renderer/translate/translate_script_browsertest.cc:115: return 0.0;
(optional) Why don't you use NAN at cmath?

https://chromiumcodereview.appspot.com/23548019/diff/10001/chrome/renderer/tr...
chrome/renderer/translate/translate_script_browsertest.cc:203: setError += "1";
This is a little uneasy to read.  How about create a function which returns an
complete set-error function?

[Takashi Toyoshima, 2013-09-09 10:07:51]

https://chromiumcodereview.appspot.com/23548019/diff/10001/chrome/renderer/tr...
File chrome/renderer/translate/translate_script_browsertest.cc (right):

https://chromiumcodereview.appspot.com/23548019/diff/10001/chrome/renderer/tr...
chrome/renderer/translate/translate_script_browsertest.cc:87: bool Error() {
On 2013/09/09 06:17:42, hajimehoshi wrote:
> GetError?

Done.
Maybe you may want to say applying the same change on ErrorCode, right?

https://chromiumcodereview.appspot.com/23548019/diff/10001/chrome/renderer/tr...
chrome/renderer/translate/translate_script_browsertest.cc:115: return 0.0;
It sounds good, but for now I'll keep it as 0.0 and add a comment to replace
this and the real implementation of TranslateHelper with NaN. I'll handle them
later in another CL since it is logically independent change.

https://chromiumcodereview.appspot.com/23548019/diff/10001/chrome/renderer/tr...
chrome/renderer/translate/translate_script_browsertest.cc:203: setError += "1";
On 2013/09/09 06:17:42, hajimehoshi wrote:
> This is a little uneasy to read.  How about create a function which returns an
> complete set-error function?

Done.

[commit-bot: I haz the power, 2013-09-09 10:08:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/toyoshim@chromium.org/23548019/17001

[commit-bot: I haz the power, 2013-09-09 15:38:40]

Change committed as 222039