Telemetry: Write chrome command line file as root if it doesn't exist.

Telemetry: Write chrome command line file as root if it doesn't exist.

On Android, Telemetry sets the chrome command line file before running any pages.
If the file doesn't already exist, it tries to write it as a normal user, rather
than root, which fails. This patch explicitly checks if the file exists. If it
doesn't exist it writes the file as root.

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

[Dominik Grewe, 2013-09-03 15:24:28]

This CL is fixing a bug recently introduced in http://crrev.com/23649003

PTAL, thanks.

[bulach, 2013-09-03 15:28:53]

lgtm, thanks! make sense to me but please let tonyg confirm his case would also
work.

[tonyg, 2013-09-03 15:44:15]

lgtm

[tonyg, 2013-09-03 15:45:48]

https://codereview.chromium.org/23726018/diff/1/tools/telemetry/telemetry/cor...
File tools/telemetry/telemetry/core/backends/chrome/android_browser_backend.py
(right):

https://codereview.chromium.org/23726018/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/backends/chrome/android_browser_backend.py:202:
if IsProtectedOrNotExistingFile(self._backend_settings.cmdline_file):
On second thought, do we need to check the parent directory permissions when the
file doesn't exist? We may be trying to create it in a root-only dir (Chrome),
or an open dir (TestShell/ContentShell).

[Dominik Grewe, 2013-09-03 15:49:06]

https://codereview.chromium.org/23726018/diff/1/tools/telemetry/telemetry/cor...
File tools/telemetry/telemetry/core/backends/chrome/android_browser_backend.py
(right):

https://codereview.chromium.org/23726018/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/backends/chrome/android_browser_backend.py:202:
if IsProtectedOrNotExistingFile(self._backend_settings.cmdline_file):
On 2013/09/03 15:45:48, tonyg wrote:
> On second thought, do we need to check the parent directory permissions when
the
> file doesn't exist? We may be trying to create it in a root-only dir (Chrome),
> or an open dir (TestShell/ContentShell).

Do we know for sure that the parent directory will exist? Otherwise we'd have to
check recursively.

[tonyg, 2013-09-03 15:50:01]

On 2013/09/03 15:49:06, Dominik Grewe wrote:
>
https://codereview.chromium.org/23726018/diff/1/tools/telemetry/telemetry/cor...
> File tools/telemetry/telemetry/core/backends/chrome/android_browser_backend.py
> (right):
> 
>
https://codereview.chromium.org/23726018/diff/1/tools/telemetry/telemetry/cor...
> tools/telemetry/telemetry/core/backends/chrome/android_browser_backend.py:202:
> if IsProtectedOrNotExistingFile(self._backend_settings.cmdline_file):
> On 2013/09/03 15:45:48, tonyg wrote:
> > On second thought, do we need to check the parent directory permissions when
> the
> > file doesn't exist? We may be trying to create it in a root-only dir
(Chrome),
> > or an open dir (TestShell/ContentShell).
> 
> Do we know for sure that the parent directory will exist? Otherwise we'd have
to
> check recursively.

Good point, I guess that we do not.

[tonyg, 2013-09-03 15:50:02]

On 2013/09/03 15:49:06, Dominik Grewe wrote:
>
https://codereview.chromium.org/23726018/diff/1/tools/telemetry/telemetry/cor...
> File tools/telemetry/telemetry/core/backends/chrome/android_browser_backend.py
> (right):
> 
>
https://codereview.chromium.org/23726018/diff/1/tools/telemetry/telemetry/cor...
> tools/telemetry/telemetry/core/backends/chrome/android_browser_backend.py:202:
> if IsProtectedOrNotExistingFile(self._backend_settings.cmdline_file):
> On 2013/09/03 15:45:48, tonyg wrote:
> > On second thought, do we need to check the parent directory permissions when
> the
> > file doesn't exist? We may be trying to create it in a root-only dir
(Chrome),
> > or an open dir (TestShell/ContentShell).
> 
> Do we know for sure that the parent directory will exist? Otherwise we'd have
to
> check recursively.

Good point, I guess that we do not.

[Dominik Grewe, 2013-09-03 16:46:41]

I've modified the method to recursively check for file permissions. Please have
a look.

[tonyg, 2013-09-04 23:40:57]

lgtm

[commit-bot: I haz the power, 2013-09-05 08:40:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dominikg@chromium.org/23726018/11001

[commit-bot: I haz the power, 2013-09-05 14:53:09]

Change committed as 221426