//content: Change ScopedTempDir::path() to GetPath()

//content: Change ScopedTempDir::path() to GetPath()

path() is being deprecated, GetPath() has better checking against wrong use.

For more context, see https://codereview.chromium.org/2275553005/.

R=jochen@chromium.org,nhiroki@chromium.org
BUG=640599
Committed: https://crrev.com/39acefd3124a956040226eea5e4c5084e64b8f01
Cr-Commit-Position: refs/heads/master@{#418229}

[vabr (Chromium), 2016-09-07 16:10:46]

Description was changed from

==========
//content: Change ScopedTempDir::path() to GetPath()

path() is being deprecated, GetPath() has better checking against wrong use.

For more context, see https://codereview.chromium.org/2275553005/.

R=jochen@chromium.org
BUG=640599
==========

to

==========
//content: Change ScopedTempDir::path() to GetPath()

path() is being deprecated, GetPath() has better checking against wrong use.

For more context, see https://codereview.chromium.org/2275553005/.

R=jochen@chromium.org
BUG=640599
==========

[vabr (Chromium), 2016-09-07 16:10:46]

vabr@chromium.org changed reviewers:
- jochen@chromium.org

[vabr (Chromium), 2016-09-08 14:01:45]

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

[commit-bot: I haz the power, 2016-09-08 14:02:11]

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-08 14:48:15]

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

[commit-bot: I haz the power, 2016-09-08 14:48:16]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[vabr (Chromium), 2016-09-08 15:24:37]

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

[commit-bot: I haz the power, 2016-09-08 15:24:53]

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-08 16:00:52]

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

[commit-bot: I haz the power, 2016-09-08 16:00:53]

Dry run: Exceeded global retry quota

[vabr (Chromium), 2016-09-09 10:58:29]

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

[commit-bot: I haz the power, 2016-09-09 10:58:48]

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-09 11:24:47]

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

[commit-bot: I haz the power, 2016-09-09 11:24:49]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[vabr (Chromium), 2016-09-09 13:35:34]

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

[commit-bot: I haz the power, 2016-09-09 13:35:42]

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-09 15:02:26]

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

[commit-bot: I haz the power, 2016-09-09 15:02:27]

Dry run: This issue passed the CQ dry run.

[vabr (Chromium), 2016-09-09 15:41:44]

vabr@chromium.org changed reviewers:
+ jochen@chromium.org, nhiroki@chromium.org

[vabr (Chromium), 2016-09-09 15:41:46]

Hi nhiroki@,

Please review my changes to content/browser/cache_storage and
content/browser/service_workers, in particular the pieces I commented on.

The code still has a need to trust that an empty path is handled correctly, I'm
merely pushing it down from ScopedTempDir to the tests using it.

Hi jochen@,

Please stamp all except content/browser/cache_storage and
content/browser/service_workers. The changes outside those two directories are
just a mechanical renaming. Which is also why I'll TBR you if nhiroki@ approves
the non-trivial part.

Thanks both!
Vaclav

https://codereview.chromium.org/2316043002/diff/120001/content/browser/cache_...
File content/browser/cache_storage/cache_storage_cache_unittest.cc (right):

https://codereview.chromium.org/2316043002/diff/120001/content/browser/cache_...
content/browser/cache_storage/cache_storage_cache_unittest.cc:334: const bool
is_incognito = MemoryOnly();
Here I forced the test to remember whether it initialised |temp_dir_| rather
than relying on |temp_dir_|'s path to give the information (which would now
DCHECK).

This does not really avoid the issue of writing files to the empty path by
accident. For that, a deeper refactoring of the related production code would be
needed (likely separate implementation of QuotaDatabase for in-memory and
on-disk storage), which seems beyond the scoped of this CL.

https://codereview.chromium.org/2316043002/diff/120001/content/browser/cache_...
File content/browser/cache_storage/cache_storage_manager_unittest.cc (right):

https://codereview.chromium.org/2316043002/diff/120001/content/browser/cache_...
content/browser/cache_storage/cache_storage_manager_unittest.cc:84: const bool
is_incognito = MemoryOnly();
The same comment as in
content/browser/cache_storage/cache_storage_cache_unittest.cc applies here.

https://codereview.chromium.org/2316043002/diff/120001/content/browser/servic...
File content/browser/service_worker/service_worker_storage_unittest.cc (right):

https://codereview.chromium.org/2316043002/diff/120001/content/browser/servic...
content/browser/service_worker/service_worker_storage_unittest.cc:299: if
(!user_data_directory_.CreateUniqueTempDir())
Also here the comment from
content/browser/cache_storage/cache_storage_cache_unittest.cc applies, changing
the reference to ServiceWorkerStorage.

[vabr (Chromium), 2016-09-09 15:42:05]

Description was changed from

==========
//content: Change ScopedTempDir::path() to GetPath()

path() is being deprecated, GetPath() has better checking against wrong use.

For more context, see https://codereview.chromium.org/2275553005/.

R=jochen@chromium.org
BUG=640599
==========

to

==========
//content: Change ScopedTempDir::path() to GetPath()

path() is being deprecated, GetPath() has better checking against wrong use.

For more context, see https://codereview.chromium.org/2275553005/.

R=jochen@chromium.org,nhiroki@chromium.org
BUG=640599
==========

[vabr (Chromium), 2016-09-09 18:00:30]

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

[commit-bot: I haz the power, 2016-09-09 18:01:03]

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-09 19:31:02]

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

[commit-bot: I haz the power, 2016-09-09 19:31:03]

Dry run: This issue passed the CQ dry run.

[nhiroki, 2016-09-12 01:50:08]

lgtm

https://codereview.chromium.org/2316043002/diff/120001/content/browser/cache_...
File content/browser/cache_storage/cache_storage_cache_unittest.cc (right):

https://codereview.chromium.org/2316043002/diff/120001/content/browser/cache_...
content/browser/cache_storage/cache_storage_cache_unittest.cc:334: const bool
is_incognito = MemoryOnly();
On 2016/09/09 15:41:46, vabr (Chromium) wrote:
> Here I forced the test to remember whether it initialised |temp_dir_| rather
> than relying on |temp_dir_|'s path to give the information (which would now
> DCHECK).
> 
> This does not really avoid the issue of writing files to the empty path by
> accident. For that, a deeper refactoring of the related production code would
be
> needed (likely separate implementation of QuotaDatabase for in-memory and
> on-disk storage), which seems beyond the scoped of this CL.

Acknowledged.

https://codereview.chromium.org/2316043002/diff/120001/content/browser/cache_...
File content/browser/cache_storage/cache_storage_manager_unittest.cc (right):

https://codereview.chromium.org/2316043002/diff/120001/content/browser/cache_...
content/browser/cache_storage/cache_storage_manager_unittest.cc:84: const bool
is_incognito = MemoryOnly();
On 2016/09/09 15:41:46, vabr (Chromium) wrote:
> The same comment as in
> content/browser/cache_storage/cache_storage_cache_unittest.cc applies here.

Acknowledged.

https://codereview.chromium.org/2316043002/diff/120001/content/browser/servic...
File content/browser/service_worker/service_worker_storage_unittest.cc (right):

https://codereview.chromium.org/2316043002/diff/120001/content/browser/servic...
content/browser/service_worker/service_worker_storage_unittest.cc:299: if
(!user_data_directory_.CreateUniqueTempDir())
On 2016/09/09 15:41:46, vabr (Chromium) wrote:
> Also here the comment from
> content/browser/cache_storage/cache_storage_cache_unittest.cc applies,
changing
> the reference to ServiceWorkerStorage.

Acknowledged.

[jochen (gone - plz use gerrit), 2016-09-12 09:58:30]

lgtm

[vabr (Chromium), 2016-09-13 12:16:48]

The CQ bit was checked by vabr@chromium.org

[vabr (Chromium), 2016-09-13 12:16:49]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
nhiroki@chromium.org
Link to the patchset: https://codereview.chromium.org/2316043002/#ps180001
(title: "Just rebased")

[commit-bot: I haz the power, 2016-09-13 12:16:59]

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-13 13:39:12]

Description was changed from

==========
//content: Change ScopedTempDir::path() to GetPath()

path() is being deprecated, GetPath() has better checking against wrong use.

For more context, see https://codereview.chromium.org/2275553005/.

R=jochen@chromium.org,nhiroki@chromium.org
BUG=640599
==========

to

==========
//content: Change ScopedTempDir::path() to GetPath()

path() is being deprecated, GetPath() has better checking against wrong use.

For more context, see https://codereview.chromium.org/2275553005/.

R=jochen@chromium.org,nhiroki@chromium.org
BUG=640599
==========

[commit-bot: I haz the power, 2016-09-13 13:39:15]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-09-13 13:40:23]

Description was changed from

==========
//content: Change ScopedTempDir::path() to GetPath()

path() is being deprecated, GetPath() has better checking against wrong use.

For more context, see https://codereview.chromium.org/2275553005/.

R=jochen@chromium.org,nhiroki@chromium.org
BUG=640599
==========

to

==========
//content: Change ScopedTempDir::path() to GetPath()

path() is being deprecated, GetPath() has better checking against wrong use.

For more context, see https://codereview.chromium.org/2275553005/.

R=jochen@chromium.org,nhiroki@chromium.org
BUG=640599

Committed: https://crrev.com/39acefd3124a956040226eea5e4c5084e64b8f01
Cr-Commit-Position: refs/heads/master@{#418229}
==========

[commit-bot: I haz the power, 2016-09-13 13:40:25]

Patchset 10 (id:??) landed as
https://crrev.com/39acefd3124a956040226eea5e4c5084e64b8f01
Cr-Commit-Position: refs/heads/master@{#418229}