content_shell/app_shell: validate --data-path and fall back to default directory

content_shell/app_shell: validate --data-path and fall back to default directory

The --data-path switch, if passed, specifies what directory to use as the browsing
context's data directory. This CL checks that the directory actually exists/can be
created, and converts relative paths to absolute.

Note: This code is executed twice in content_shell (for normal and OTR contexts); in
the long run, giving the shell its own PathProvider may be a better solution.

Committed: https://crrev.com/49a54b594e0c9f6d3ea60d1f21e90507c21423e2
Cr-Commit-Position: refs/heads/master@{#422670}

[michaelpg, 2016-09-16 01:09:43]

The CQ bit was checked by michaelpg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-16 01:10:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-16 01:17:56]

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

[commit-bot: I haz the power, 2016-09-16 01:17:57]

Dry run: Try jobs failed on following builders:
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[michaelpg, 2016-09-16 21:55:13]

Description was changed from

==========
Allow relative --data-path in content_shell/app_shell and fall back to default
directory

BUG=
==========

to

==========
Allow relative --data-path in content_shell/app_shell; fall back to default
directory

The --data-path switch, if passed, specifies what directory to use as the
browsing
context's data directory. This CL checks that the directory actually exists/can
be
created, and converts relative paths to absolute.

Note: This code is executed twice in content_shell (for normal and OTR
contexts); in
the long run, giving the shell its own PathProvider may be a better solution.
==========

[michaelpg, 2016-09-16 21:57:03]

Description was changed from

==========
Allow relative --data-path in content_shell/app_shell; fall back to default
directory

The --data-path switch, if passed, specifies what directory to use as the
browsing
context's data directory. This CL checks that the directory actually exists/can
be
created, and converts relative paths to absolute.

Note: This code is executed twice in content_shell (for normal and OTR
contexts); in
the long run, giving the shell its own PathProvider may be a better solution.
==========

to

==========
content_shell/app_shell: validate --data-path and fall back to default directory

The --data-path switch, if passed, specifies what directory to use as the
browsing
context's data directory. This CL checks that the directory actually exists/can
be
created, and converts relative paths to absolute.

Note: This code is executed twice in content_shell (for normal and OTR
contexts); in
the long run, giving the shell its own PathProvider may be a better solution.
==========

[michaelpg, 2016-09-16 22:02:55]

michaelpg@chromium.org changed reviewers:
+ mkwst@chromium.org

[michaelpg, 2016-09-16 22:02:56]

Mike, does this look OK to you? Besides making developer's (read: my) lives
easier, could this have unwanted effects I'm not seeing?

[michaelpg, 2016-09-29 01:33:07]

michaelpg@chromium.org changed reviewers:
+ peter@chromium.org

[michaelpg, 2016-09-29 01:33:08]

Peter, would you mind taking a look?

[Peter Beverloo, 2016-09-29 16:22:53]

lgtm

https://codereview.chromium.org/2344073003/diff/40001/content/shell/browser/s...
File content/shell/browser/shell_browser_context.cc (right):

https://codereview.chromium.org/2344073003/diff/40001/content/shell/browser/s...
content/shell/browser/shell_browser_context.cc:79: // We need to have an
absolute path.
nit: might be good to document that the PathService normally takes care of this,
and features therefore might assume this to be the case.

[michaelpg, 2016-09-30 03:38:56]

Thanks!

https://codereview.chromium.org/2344073003/diff/40001/content/shell/browser/s...
File content/shell/browser/shell_browser_context.cc (right):

https://codereview.chromium.org/2344073003/diff/40001/content/shell/browser/s...
content/shell/browser/shell_browser_context.cc:79: // We need to have an
absolute path.
On 2016/09/29 16:22:53, Peter Beverloo wrote:
> nit: might be good to document that the PathService normally takes care of
this,
> and features therefore might assume this to be the case.

Done.

[michaelpg, 2016-09-30 03:39:06]

michaelpg@chromium.org changed reviewers:
- mkwst@chromium.org

[michaelpg, 2016-10-04 00:57:58]

The CQ bit was checked by michaelpg@chromium.org

[michaelpg, 2016-10-04 00:57:59]

The patchset sent to the CQ was uploaded after l-g-t-m from peter@chromium.org
Link to the patchset: https://codereview.chromium.org/2344073003/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2016-10-04 00:58:27]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-04 02:07:45]

Description was changed from

==========
content_shell/app_shell: validate --data-path and fall back to default directory

The --data-path switch, if passed, specifies what directory to use as the
browsing
context's data directory. This CL checks that the directory actually exists/can
be
created, and converts relative paths to absolute.

Note: This code is executed twice in content_shell (for normal and OTR
contexts); in
the long run, giving the shell its own PathProvider may be a better solution.
==========

to

==========
content_shell/app_shell: validate --data-path and fall back to default directory

The --data-path switch, if passed, specifies what directory to use as the
browsing
context's data directory. This CL checks that the directory actually exists/can
be
created, and converts relative paths to absolute.

Note: This code is executed twice in content_shell (for normal and OTR
contexts); in
the long run, giving the shell its own PathProvider may be a better solution.
==========

[commit-bot: I haz the power, 2016-10-04 02:07:46]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-10-04 02:11:48]

Description was changed from

==========
content_shell/app_shell: validate --data-path and fall back to default directory

The --data-path switch, if passed, specifies what directory to use as the
browsing
context's data directory. This CL checks that the directory actually exists/can
be
created, and converts relative paths to absolute.

Note: This code is executed twice in content_shell (for normal and OTR
contexts); in
the long run, giving the shell its own PathProvider may be a better solution.
==========

to

==========
content_shell/app_shell: validate --data-path and fall back to default directory

The --data-path switch, if passed, specifies what directory to use as the
browsing
context's data directory. This CL checks that the directory actually exists/can
be
created, and converts relative paths to absolute.

Note: This code is executed twice in content_shell (for normal and OTR
contexts); in
the long run, giving the shell its own PathProvider may be a better solution.

Committed: https://crrev.com/49a54b594e0c9f6d3ea60d1f21e90507c21423e2
Cr-Commit-Position: refs/heads/master@{#422670}
==========

[commit-bot: I haz the power, 2016-10-04 02:11:49]

Patchset 5 (id:??) landed as
https://crrev.com/49a54b594e0c9f6d3ea60d1f21e90507c21423e2
Cr-Commit-Position: refs/heads/master@{#422670}