Use base::File for DebugDaemonClient::GetDebugLogs

Use base::File for DebugDaemonClient::GetDebugLogs

There is no need to keep the file descriptor open in net_internals_ui.cc as libdbus duplicates the FD when appropriate.

Use base::File instead of PlatformFile.
Use blocking pool instead of WorkerPool.

BUG=369183
TEST=Can save debug logs in "Chrome OS" section of chrome://net-internals on a Chromebook.

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

[satorux1, 2014-05-08 09:07:33]

chromeos LGTM

[mmenke, 2014-05-08 14:29:33]

https://codereview.chromium.org/270853002/diff/1/chrome/browser/ui/webui/net_...
File chrome/browser/ui/webui/net_internals/net_internals_ui.cc (right):

https://codereview.chromium.org/270853002/diff/1/chrome/browser/ui/webui/net_...
chrome/browser/ui/webui/net_internals/net_internals_ui.cc:251: file->Pass(),
Erm...I don't see Pass() defined anywhere for a base::File.

[hashimoto, 2014-05-08 15:13:12]

https://codereview.chromium.org/270853002/diff/1/chrome/browser/ui/webui/net_...
File chrome/browser/ui/webui/net_internals/net_internals_ui.cc (right):

https://codereview.chromium.org/270853002/diff/1/chrome/browser/ui/webui/net_...
chrome/browser/ui/webui/net_internals/net_internals_ui.cc:251: file->Pass(),
On 2014/05/08 14:29:34, mmenke wrote:
> Erm...I don't see Pass() defined anywhere for a base::File.

Like scoped_ptr, base::File contains MOVE_ONLY_TYPE_FOR_CPP_03 in its class
definition so it comes with Pass() method somehow implicitly.

[mmenke, 2014-05-08 15:43:50]

https://codereview.chromium.org/270853002/diff/1/chrome/browser/ui/webui/net_...
File chrome/browser/ui/webui/net_internals/net_internals_ui.cc (right):

https://codereview.chromium.org/270853002/diff/1/chrome/browser/ui/webui/net_...
chrome/browser/ui/webui/net_internals/net_internals_ui.cc:251: file->Pass(),
On 2014/05/08 15:13:12, hashimoto wrote:
> On 2014/05/08 14:29:34, mmenke wrote:
> > Erm...I don't see Pass() defined anywhere for a base::File.
> 
> Like scoped_ptr, base::File contains MOVE_ONLY_TYPE_FOR_CPP_03 in its class
> definition so it comes with Pass() method somehow implicitly.

I'm having trouble understanding their explanation, but it looks to me like
we're leaking the file* (Though we are moving its internals correctly).  It
should be passed as a base::Owned, not base::Unretained, no?

[hashimoto, 2014-05-08 15:59:51]

https://codereview.chromium.org/270853002/diff/1/chrome/browser/ui/webui/net_...
File chrome/browser/ui/webui/net_internals/net_internals_ui.cc (right):

https://codereview.chromium.org/270853002/diff/1/chrome/browser/ui/webui/net_...
chrome/browser/ui/webui/net_internals/net_internals_ui.cc:251: file->Pass(),
On 2014/05/08 15:43:50, mmenke wrote:
> On 2014/05/08 15:13:12, hashimoto wrote:
> > On 2014/05/08 14:29:34, mmenke wrote:
> > > Erm...I don't see Pass() defined anywhere for a base::File.
> > 
> > Like scoped_ptr, base::File contains MOVE_ONLY_TYPE_FOR_CPP_03 in its class
> > definition so it comes with Pass() method somehow implicitly.
> 
> I'm having trouble understanding their explanation, but it looks to me like
> we're leaking the file* (Though we are moving its internals correctly).  It
> should be passed as a base::Owned, not base::Unretained, no?

StoreDebugLogs() below runs base::File::Initialize() on the blocking pool with
base::Unretained and runs WriteDebugLogToFile() on the UI thread with the
initialized base::File passed with base::Owned.
base::File object is kept alive until WriteDebugLogToFile() finishes and it gets
correctly destroyed after that.

[mmenke, 2014-05-08 16:06:11]

On 2014/05/08 15:59:51, hashimoto wrote:
>
https://codereview.chromium.org/270853002/diff/1/chrome/browser/ui/webui/net_...
> File chrome/browser/ui/webui/net_internals/net_internals_ui.cc (right):
> 
>
https://codereview.chromium.org/270853002/diff/1/chrome/browser/ui/webui/net_...
> chrome/browser/ui/webui/net_internals/net_internals_ui.cc:251: file->Pass(),
> On 2014/05/08 15:43:50, mmenke wrote:
> > On 2014/05/08 15:13:12, hashimoto wrote:
> > > On 2014/05/08 14:29:34, mmenke wrote:
> > > > Erm...I don't see Pass() defined anywhere for a base::File.
> > > 
> > > Like scoped_ptr, base::File contains MOVE_ONLY_TYPE_FOR_CPP_03 in its
class
> > > definition so it comes with Pass() method somehow implicitly.
> > 
> > I'm having trouble understanding their explanation, but it looks to me like
> > we're leaking the file* (Though we are moving its internals correctly).  It
> > should be passed as a base::Owned, not base::Unretained, no?
> 
> StoreDebugLogs() below runs base::File::Initialize() on the blocking pool with
> base::Unretained and runs WriteDebugLogToFile() on the UI thread with the
> initialized base::File passed with base::Owned.
> base::File object is kept alive until WriteDebugLogToFile() finishes and it
gets
> correctly destroyed after that.

Ah, right.  LGTM.

[hashimoto, 2014-05-09 05:04:29]

The CQ bit was checked by hashimoto@chromium.org

[commit-bot: I haz the power, 2014-05-09 05:09:08]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hashimoto@chromium.org/270853002/1

[commit-bot: I haz the power, 2014-05-10 06:19:20]

Change committed as 269540