Chromoting browser-tests should be able to read files specified with relative paths.

Use ReadFile instead of ReadFileToString, to work with relative paths.

BUG=424844
Committed: https://crrev.com/aa3e63da0ed80c29d46a92a545560f6515ad652e
Cr-Commit-Position: refs/heads/master@{#300834}

[anandc, 2014-10-18 02:33:45]

anandc@chromium.org changed reviewers:
+ weitaosu@chromium.org

[anandc, 2014-10-18 02:33:45]

Hi Weitao,

Please take a look. 
An error reading contents of the test file was encountered when attempting to
run the Auth test for the first time using the isolate+swarm system. 
The fix made results in the Auth test succeeding.

https://codereview.chromium.org/639123010/diff/1/chrome/test/remoting/remote_...
File chrome/test/remoting/remote_desktop_browsertest.cc (left):

https://codereview.chromium.org/639123010/diff/1/chrome/test/remoting/remote_...
chrome/test/remoting/remote_desktop_browsertest.cc:826:
ASSERT_TRUE(base::ReadFileToString(accounts_file_path, &accounts_info));
ReadFileToString returns an error if the file-path is relative. 
See comment here:
https://code.google.com/p/chromium/codesearch#chromium/src/base/files/file_ut...
Tests that run on the swarming slaves use relative paths, so we have to use
ReadFile instead.

[anandc, 2014-10-20 18:48:49]

anandc@chromium.org changed reviewers:
+ garykac@chromium.org

[anandc, 2014-10-20 18:48:50]

Adding garykac@ to reviewers, who offered to take a look as well.

Thanks.

[anandc, 2014-10-21 15:43:41]

On 2014/10/20 18:48:50, anandc wrote:
> Adding garykac@ to reviewers, who offered to take a look as well.
> 
> Thanks.

Ping.

[anandc, 2014-10-22 15:45:46]

anandc@chromium.org changed reviewers:
- garykac@chromium.org

[anandc, 2014-10-22 15:46:07]

On 2014/10/21 15:43:41, anandc wrote:
> On 2014/10/20 18:48:50, anandc wrote:
> > Adding garykac@ to reviewers, who offered to take a look as well.
> > 
> > Thanks.
> 
> Ping.

Another ping. Weitao, PTAL. 
Thanks.
(Removed Gary, to not clutter reviewers.)

[Jamie, 2014-10-22 23:02:26]

jamiewalch@chromium.org changed reviewers:
+ jamiewalch@chromium.org

[Jamie, 2014-10-22 23:02:27]

ltgm once my comment is addressed.

https://codereview.chromium.org/639123010/diff/1/chrome/test/remoting/remote_...
File chrome/test/remoting/remote_desktop_browsertest.cc (right):

https://codereview.chromium.org/639123010/diff/1/chrome/test/remoting/remote_...
chrome/test/remoting/remote_desktop_browsertest.cc:825: // To handle relative
paths, we'll first get the size of the file.
Getting the size of the file is not only needed for relative file paths, so this
comment is a bit confusing. I think the comment you added to the code-review is
better and would explain why we can't use ReadFileToString here.

https://codereview.chromium.org/639123010/diff/1/chrome/test/remoting/remote_...
chrome/test/remoting/remote_desktop_browsertest.cc:830:
ASSERT_FALSE(base::ReadFile(accounts_file_path, buf, sizeof(buf)-1) == -1);
Nit: spaces around the subtraction operator.

[anandc, 2014-10-22 23:59:35]

Patchset #2 (id:20001) has been deleted

[anandc, 2014-10-23 00:00:40]

Thanks. Done.

https://codereview.chromium.org/639123010/diff/1/chrome/test/remoting/remote_...
File chrome/test/remoting/remote_desktop_browsertest.cc (right):

https://codereview.chromium.org/639123010/diff/1/chrome/test/remoting/remote_...
chrome/test/remoting/remote_desktop_browsertest.cc:825: // To handle relative
paths, we'll first get the size of the file.
On 2014/10/22 23:02:26, Jamie wrote:
> Getting the size of the file is not only needed for relative file paths, so
this
> comment is a bit confusing. I think the comment you added to the code-review
is
> better and would explain why we can't use ReadFileToString here.

Done.

https://codereview.chromium.org/639123010/diff/1/chrome/test/remoting/remote_...
chrome/test/remoting/remote_desktop_browsertest.cc:830:
ASSERT_FALSE(base::ReadFile(accounts_file_path, buf, sizeof(buf)-1) == -1);
On 2014/10/22 23:02:26, Jamie wrote:
> Nit: spaces around the subtraction operator.

Done.

[anandc, 2014-10-23 00:00:45]

The CQ bit was checked by anandc@chromium.org

[commit-bot: I haz the power, 2014-10-23 00:02:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/639123010/40001

[commit-bot: I haz the power, 2014-10-23 00:02:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-23 00:02:47]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[Jamie, 2014-10-23 00:27:59]

Probably because I spelled "LGTM" wrong :)

[Jamie, 2014-10-23 00:28:03]

The CQ bit was checked by jamiewalch@chromium.org

[chromium-reviews, 2014-10-23 00:28:45]

oh, didn't notice, kept wondering why it is not green. Thanks.

On Wed, Oct 22, 2014 at 5:27 PM, <jamiewalch@chromium.org> wrote:

> Probably because I spelled "LGTM" wrong :)
>
> https://codereview.chromium.org/639123010/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2014-10-23 00:29:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/639123010/40001

[commit-bot: I haz the power, 2014-10-23 01:21:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-23 01:21:27]

Try jobs failed on following builders:
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)

[anandc, 2014-10-23 03:27:18]

The CQ bit was checked by anandc@chromium.org

[commit-bot: I haz the power, 2014-10-23 03:28:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/639123010/60001

[commit-bot: I haz the power, 2014-10-23 04:15:36]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2014-10-23 04:16:17]

Patchset 3 (id:??) landed as
https://crrev.com/aa3e63da0ed80c29d46a92a545560f6515ad652e
Cr-Commit-Position: refs/heads/master@{#300834}