Allow temporary file system for incognito DevTools.

Allow temporary file system for incognito DevTools.

It's an essential part of Timeline which should work in incognito mode.

BUG=490148
Committed: https://crrev.com/3caf87e1c558751ffd82cd4d7df57d63a0462a06
Cr-Commit-Position: refs/heads/master@{#437495}

[dgozman, 2016-12-06 20:00:29]

dgozman@chromium.org changed reviewers:
+ caseq@chromium.org, nhiroki@chromium.org, pfeldman@chromium.org

[dgozman, 2016-12-06 20:00:29]

Could you please take a look?

[pfeldman, 2016-12-06 20:13:41]

lgtm

[nhiroki, 2016-12-07 07:38:45]

Description was changed from

==========
Allow temporary file system for incognito DevTools.

It's an essential part of Timeline which should work in incognito mode.

BUG=490148
==========

to

==========
Allow temporary file system for incognito DevTools.

It's an essential part of Timeline which should work in incognito mode.

BUG=490148
==========

[nhiroki, 2016-12-07 07:38:45]

nhiroki@chromium.org changed reviewers:
+ jsbell@chromium.org

[nhiroki, 2016-12-07 07:40:49]

In my understanding, Filesystem API on incognito mode hasn't been supported
other than ChromeOS:
https://bugs.chromium.org/p/chromium/issues/detail?id=93417

+jsbell@, is this correct?

[dgozman, 2016-12-07 09:20:00]

On 2016/12/07 07:40:49, nhiroki wrote:
> In my understanding, Filesystem API on incognito mode hasn't been supported
> other than ChromeOS:
> https://bugs.chromium.org/p/chromium/issues/detail?id=93417
> 
> +jsbell@, is this correct?

Apparently, it works if you allow it, by using the real (not in-memory) backend.
For DevTools, we think it's fine to do so, as we guarantee that's user's
intention.
Same thing is done on ChromeOS for some extensions (they use the same flag).

[dgozman, 2016-12-08 19:48:49]

@nhiroki, @jsbell: friendly ping.

[kinuko, 2016-12-09 03:37:15]

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

[kinuko, 2016-12-09 03:37:15]

https://codereview.chromium.org/2556803003/diff/1/chrome/browser/ui/webui/dev...
File chrome/browser/ui/webui/devtools_ui.cc (right):

https://codereview.chromium.org/2556803003/diff/1/chrome/browser/ui/webui/dev...
chrome/browser/ui/webui/devtools_ui.cc:448: GURL site =
content::SiteInstance::GetSiteForURL(profile, url);
Just to be extra sure, this URL is going to be devtools specific one, is that
right?

[kinuko, 2016-12-09 03:37:55]

On 2016/12/09 03:37:15, kinuko wrote:
>
https://codereview.chromium.org/2556803003/diff/1/chrome/browser/ui/webui/dev...
> File chrome/browser/ui/webui/devtools_ui.cc (right):
> 
>
https://codereview.chromium.org/2556803003/diff/1/chrome/browser/ui/webui/dev...
> chrome/browser/ui/webui/devtools_ui.cc:448: GURL site =
> content::SiteInstance::GetSiteForURL(profile, url);
> Just to be extra sure, this URL is going to be devtools specific one, is that
> right?

If above is true this lgtm

[dgozman, 2016-12-09 05:51:47]

Thank you for review.

https://codereview.chromium.org/2556803003/diff/1/chrome/browser/ui/webui/dev...
File chrome/browser/ui/webui/devtools_ui.cc (right):

https://codereview.chromium.org/2556803003/diff/1/chrome/browser/ui/webui/dev...
chrome/browser/ui/webui/devtools_ui.cc:448: GURL site =
content::SiteInstance::GetSiteForURL(profile, url);
On 2016/12/09 03:37:15, kinuko wrote:
> Just to be extra sure, this URL is going to be devtools specific one, is that
> right?

Correct. We even sanitize it in line 444.

[dgozman, 2016-12-09 05:51:51]

The CQ bit was checked by dgozman@chromium.org

[commit-bot: I haz the power, 2016-12-09 05:52:13]

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

[nhiroki, 2016-12-09 07:52:59]

Sorry for my delayed reply. LGTM

https://codereview.chromium.org/2556803003/diff/1/storage/browser/fileapi/fil...
File storage/browser/fileapi/file_system_context.h (left):

https://codereview.chromium.org/2556803003/diff/1/storage/browser/fileapi/fil...
storage/browser/fileapi/file_system_context.h:293: // Used only on ChromeOS for
now.
Could you add a comment like this?:

// Used only on ChromeOS or for DevTools on other platforms.
// (https://crbug.com/93417)

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

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1481262711711700, "parent_rev":
"26cfd73083b63b2c865e6554d7857063e3d63b05", "commit_rev":
"1653de3e583c6cfe641b550d10420f7fe9ba7d35"}

[commit-bot: I haz the power, 2016-12-09 08:13:51]

Description was changed from

==========
Allow temporary file system for incognito DevTools.

It's an essential part of Timeline which should work in incognito mode.

BUG=490148
==========

to

==========
Allow temporary file system for incognito DevTools.

It's an essential part of Timeline which should work in incognito mode.

BUG=490148
==========

[commit-bot: I haz the power, 2016-12-09 08:13:52]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-12-09 08:15:36]

Description was changed from

==========
Allow temporary file system for incognito DevTools.

It's an essential part of Timeline which should work in incognito mode.

BUG=490148
==========

to

==========
Allow temporary file system for incognito DevTools.

It's an essential part of Timeline which should work in incognito mode.

BUG=490148

Committed: https://crrev.com/3caf87e1c558751ffd82cd4d7df57d63a0462a06
Cr-Commit-Position: refs/heads/master@{#437495}
==========

[commit-bot: I haz the power, 2016-12-09 08:15:37]

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