Pepper: Move FileIO host from renderer to browser.

Pepper: Move FileIO host from renderer to browser.

This change is large because it moves QuotaFileIO and PepperFileIOHost all at
once to the browser process. Some code in the refactored PepperFileIOHost is
moved from what's provided in FileAPIMessageFilter.

Tested locally with Bastion, From Dust, and Angry Bots.

TBR=jschuh
BUG=246396
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=232440

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

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

[bbudge, 2013-10-22 01:59:38]

Comments so far. I'll make another pass on the new CL soon.

https://codereview.chromium.org/33053002/diff/1/content/browser/renderer_host...
File content/browser/renderer_host/pepper/pepper_file_io_host.cc (right):

https://codereview.chromium.org/33053002/diff/1/content/browser/renderer_host...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:46: // things
depending on whether is negative or not.  We translate for those
s/is/it is

https://codereview.chromium.org/33053002/diff/1/content/browser/renderer_host...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:266:
file_system_url_.type()) == NULL) {
nit: !expr seems more consistent with the rest of the file than "== NULL".

https://codereview.chromium.org/33053002/diff/1/content/browser/renderer_host...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:289:
weak_factory_.GetWeakPtr(),
nit: Indent is different from other base::Bind invocations in the file.

https://codereview.chromium.org/33053002/diff/1/content/browser/renderer_host...
File content/browser/renderer_host/pepper/pepper_file_io_host.h (right):

https://codereview.chromium.org/33053002/diff/1/content/browser/renderer_host...
content/browser/renderer_host/pepper/pepper_file_io_host.h:49: // Stable API
Message Handlers.
I don't understand the comment.

https://codereview.chromium.org/33053002/diff/1/content/browser/renderer_host...
File content/browser/renderer_host/pepper/pepper_file_ref_host.h (right):

https://codereview.chromium.org/33053002/diff/1/content/browser/renderer_host...
content/browser/renderer_host/pepper/pepper_file_ref_host.h:106:
base::WeakPtr<PepperFileSystemBrowserHost> file_system_host_;
It looks like this is only used in a single method, which passes it to the
backend. Is there a reason to hold onto the weak ptr here?

[teravest, 2013-10-22 15:45:05]

https://codereview.chromium.org/33053002/diff/1/content/browser/renderer_host...
File content/browser/renderer_host/pepper/pepper_file_io_host.cc (right):

https://codereview.chromium.org/33053002/diff/1/content/browser/renderer_host...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:46: // things
depending on whether is negative or not.  We translate for those
On 2013/10/22 01:59:39, bbudge1 wrote:
> s/is/it is

Done.

https://codereview.chromium.org/33053002/diff/1/content/browser/renderer_host...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:266:
file_system_url_.type()) == NULL) {
On 2013/10/22 01:59:39, bbudge1 wrote:
> nit: !expr seems more consistent with the rest of the file than "== NULL".

Done.

https://codereview.chromium.org/33053002/diff/1/content/browser/renderer_host...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:289:
weak_factory_.GetWeakPtr(),
On 2013/10/22 01:59:39, bbudge1 wrote:
> nit: Indent is different from other base::Bind invocations in the file.

Done.

https://codereview.chromium.org/33053002/diff/1/content/browser/renderer_host...
File content/browser/renderer_host/pepper/pepper_file_io_host.h (right):

https://codereview.chromium.org/33053002/diff/1/content/browser/renderer_host...
content/browser/renderer_host/pepper/pepper_file_io_host.h:49: // Stable API
Message Handlers.
On 2013/10/22 01:59:39, bbudge1 wrote:
> I don't understand the comment.

OK, Removed (and removed an undefined method).

https://codereview.chromium.org/33053002/diff/1/content/browser/renderer_host...
File content/browser/renderer_host/pepper/pepper_file_ref_host.h (right):

https://codereview.chromium.org/33053002/diff/1/content/browser/renderer_host...
content/browser/renderer_host/pepper/pepper_file_ref_host.h:106:
base::WeakPtr<PepperFileSystemBrowserHost> file_system_host_;
On 2013/10/22 01:59:39, bbudge1 wrote:
> It looks like this is only used in a single method, which passes it to the
> backend. Is there a reason to hold onto the weak ptr here?

Good catch, removed.

[teravest, 2013-10-22 16:01:59]

+tzik for webkit/browser/fileapi
+joi for content/public
+kinuko for content/child/fileapi and content/common/fileapi

[Jói, 2013-10-22 16:23:52]

//content/public LGTM.

[bbudge, 2013-10-22 17:25:36]

A few more things. Looking good.

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
File content/browser/renderer_host/pepper/pepper_file_io_host.cc (right):

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:10: #include
"base/command_line.h"
Is this needed?

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:133:
RenderProcessHost* render_process_host =
Maybe change variable name to 'host' as in PepperFileSystemBrowserHost?

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:168:
OnHostMsgClose(NULL);
It might be cleaner to define a Close() method to be called here and from
OnHostMsgClose.

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:297:
on_close_callback_ = on_close_callback;
on_close_callback_ doesn't seem to be used anywhere else.

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:369:
context->MakeReplyMessageContext());
Could you factor out creating the callback in OnHostMsgWrite like you do here?

[kinuko, 2013-10-23 07:16:04]

https://codereview.chromium.org/33053002/diff/230001/content/browser/fileapi/...
File content/browser/fileapi/fileapi_message_filter.cc (right):

https://codereview.chromium.org/33053002/diff/230001/content/browser/fileapi/...
content/browser/fileapi/fileapi_message_filter.cc:513: }
This logic can be probably removed (and we need to make sure we keep the
semantics in the new code)

https://codereview.chromium.org/33053002/diff/230001/content/browser/fileapi/...
File content/browser/fileapi/fileapi_message_filter.h (right):

https://codereview.chromium.org/33053002/diff/230001/content/browser/fileapi/...
content/browser/fileapi/fileapi_message_filter.h:130: void OnNotifyCloseFile(int
file_open_id);
This could probably be removed too

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
File content/browser/renderer_host/pepper/pepper_file_io_host.cc (right):

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:407:
base::ResetAndReturn(&notify_close_file_callback_));
No one sets this callback any more?

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:410: }
Please call on_close_callback_ here if it's non-null.

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:434:
browser_ppapi_host_->GetDocumentURLForInstance(pp_instance()))) {
Instead of returning raw render_process_host ptr from UI to IO thread (which I
don't feel quite right) can we call this on UI thread and simply return the
boolean result value to IO thread?

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:504: if
(render_process_host_) {
We're caching this raw ptr on IO thread while it's basically resident of UI
thread. I don't think this is safe, is it?

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
File content/browser/renderer_host/pepper/pepper_file_io_host.h (right):

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.h:86:
scoped_refptr<fileapi::FileSystemContext> fs_context);
nit: indent

https://codereview.chromium.org/33053002/diff/230001/content/child/fileapi/fi...
File content/child/fileapi/file_system_dispatcher.h (right):

https://codereview.chromium.org/33053002/diff/230001/content/child/fileapi/fi...
content/child/fileapi/file_system_dispatcher.h:131: void NotifyCloseFile(int
file_open_id);
This can probably be removed too

https://codereview.chromium.org/33053002/diff/230001/content/common/fileapi/f...
File content/common/fileapi/file_system_messages.h (right):

https://codereview.chromium.org/33053002/diff/230001/content/common/fileapi/f...
content/common/fileapi/file_system_messages.h:169: int /* file_open_id */)
This can probably be removed

[tzik, 2013-10-23 09:28:50]

webkit/browser/fileapi changes LGTM

[teravest, 2013-10-23 16:54:27]

https://codereview.chromium.org/33053002/diff/230001/content/browser/fileapi/...
File content/browser/fileapi/fileapi_message_filter.cc (right):

https://codereview.chromium.org/33053002/diff/230001/content/browser/fileapi/...
content/browser/fileapi/fileapi_message_filter.cc:513: }
On 2013/10/23 07:16:04, kinuko wrote:
> This logic can be probably removed (and we need to make sure we keep the
> semantics in the new code)

Done.

https://codereview.chromium.org/33053002/diff/230001/content/browser/fileapi/...
File content/browser/fileapi/fileapi_message_filter.h (right):

https://codereview.chromium.org/33053002/diff/230001/content/browser/fileapi/...
content/browser/fileapi/fileapi_message_filter.h:130: void OnNotifyCloseFile(int
file_open_id);
On 2013/10/23 07:16:04, kinuko wrote:
> This could probably be removed too

Done.

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
File content/browser/renderer_host/pepper/pepper_file_io_host.cc (right):

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:10: #include
"base/command_line.h"
On 2013/10/22 17:25:37, bbudge1 wrote:
> Is this needed?

Removed.

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:133:
RenderProcessHost* render_process_host =
On 2013/10/22 17:25:37, bbudge1 wrote:
> Maybe change variable name to 'host' as in PepperFileSystemBrowserHost?

Done.

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:168:
OnHostMsgClose(NULL);
On 2013/10/22 17:25:37, bbudge1 wrote:
> It might be cleaner to define a Close() method to be called here and from
> OnHostMsgClose.

I'll change this if you feel strongly, but I don't think it would accomplish
much.

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:297:
on_close_callback_ = on_close_callback;
On 2013/10/22 17:25:37, bbudge1 wrote:
> on_close_callback_ doesn't seem to be used anywhere else.

Fixed.

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:369:
context->MakeReplyMessageContext());
On 2013/10/22 17:25:37, bbudge1 wrote:
> Could you factor out creating the callback in OnHostMsgWrite like you do here?

Done.

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:407:
base::ResetAndReturn(&notify_close_file_callback_));
On 2013/10/23 07:16:04, kinuko wrote:
> No one sets this callback any more?

Fixed.

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:410: }
On 2013/10/23 07:16:04, kinuko wrote:
> Please call on_close_callback_ here if it's non-null.

Done. Thanks for catching this. I changed this slightly to call
on_close_callback_ once base::FileUtilProxy::Close has finished.

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:434:
browser_ppapi_host_->GetDocumentURLForInstance(pp_instance()))) {
On 2013/10/23 07:16:04, kinuko wrote:
> Instead of returning raw render_process_host ptr from UI to IO thread (which I
> don't feel quite right) can we call this on UI thread and simply return the
> boolean result value to IO thread?

The problem is that BrowserPpapiHost lives entirely on the I/O thread.
I suppose I could get the BrowserContext on the UI thread instead of the
RenderProcessHost, but I'm not sure that's much better.

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:504: if
(render_process_host_) {
On 2013/10/23 07:16:04, kinuko wrote:
> We're caching this raw ptr on IO thread while it's basically resident of UI
> thread. I don't think this is safe, is it?

I was thinking that RenderProcessHost::FromID was restricted to the UI thread
(since g_all_hosts isn't thread safe). I don't see any other thread restrictions
in RenderProcessHost, though.

Am I missing something?

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
File content/browser/renderer_host/pepper/pepper_file_io_host.h (right):

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.h:86:
scoped_refptr<fileapi::FileSystemContext> fs_context);
On 2013/10/23 07:16:04, kinuko wrote:
> nit: indent

Done.

https://codereview.chromium.org/33053002/diff/230001/content/child/fileapi/fi...
File content/child/fileapi/file_system_dispatcher.h (right):

https://codereview.chromium.org/33053002/diff/230001/content/child/fileapi/fi...
content/child/fileapi/file_system_dispatcher.h:131: void NotifyCloseFile(int
file_open_id);
On 2013/10/23 07:16:04, kinuko wrote:
> This can probably be removed too

Done.

https://codereview.chromium.org/33053002/diff/230001/content/common/fileapi/f...
File content/common/fileapi/file_system_messages.h (right):

https://codereview.chromium.org/33053002/diff/230001/content/common/fileapi/f...
content/common/fileapi/file_system_messages.h:169: int /* file_open_id */)
On 2013/10/23 07:16:04, kinuko wrote:
> This can probably be removed

Done.

[bbudge, 2013-10-23 17:04:52]

LGTM

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
File content/browser/renderer_host/pepper/pepper_file_io_host.cc (right):

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:168:
OnHostMsgClose(NULL);
On 2013/10/23 16:54:27, teravest wrote:
> On 2013/10/22 17:25:37, bbudge1 wrote:
> > It might be cleaner to define a Close() method to be called here and from
> > OnHostMsgClose.
> 
> I'll change this if you feel strongly, but I don't think it would accomplish
> much.

OK

[teravest, 2013-10-23 17:52:04]

+jochen for chrome/renderer/*, content/content_renderer.gypi,
content/content_tests.gypi

+jschuh for content/common/view_messages.h,
content/common/fileapi/file_system_messages.h

+jam for content/content_browser.gypi

[jam, 2013-10-24 00:31:18]

On 2013/10/23 17:52:04, teravest wrote:
> +jochen for chrome/renderer/*, content/content_renderer.gypi,
> content/content_tests.gypi
> 
> +jschuh for content/common/view_messages.h,
> content/common/fileapi/file_system_messages.h
> 
> +jam for content/content_browser.gypi

lgtm for all the gpii files

[kinuko, 2013-10-24 02:31:00]

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
File content/browser/renderer_host/pepper/pepper_file_io_host.cc (right):

https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
content/browser/renderer_host/pepper/pepper_file_io_host.cc:434:
browser_ppapi_host_->GetDocumentURLForInstance(pp_instance()))) {
On 2013/10/23 16:54:27, teravest wrote:
> On 2013/10/23 07:16:04, kinuko wrote:
> > Instead of returning raw render_process_host ptr from UI to IO thread (which
I
> > don't feel quite right) can we call this on UI thread and simply return the
> > boolean result value to IO thread?
> 
> The problem is that BrowserPpapiHost lives entirely on the I/O thread.
> I suppose I could get the BrowserContext on the UI thread instead of the
> RenderProcessHost, but I'm not sure that's much better.

I see. Could we do this then: call BrowserPpapiHost::GetDocumentURLForInstance()
on the IO thread first, pass the document URL to the UI thread to check if it's
allowed and return back the result to IO thread.

As you wrote below RPH doesn't have explicit thread restrictions on some
methods, but I think our usual assumption is: when it's not explicitly noted
that it's thread-safe it tends not to be so. (You can take a look at RPH code
and how most of methods are implemented without thread restrictions but in a
non-thread-safe way)  GetBrowserContext() looks safe, but we need to make double
sure that PepperFileIOHost never outlives RPH and GetBrowserContext keeps to be
thread-safe.  I'd like to avoid adding such implicit assumptions there.

[jochen (gone - plz use gerrit), 2013-10-24 12:41:07]

chrome/renderer lgtm

[teravest1, 2013-10-24 14:45:42]

On Wed, Oct 23, 2013 at 8:31 PM,  <kinuko@chromium.org> wrote:
>
>
https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
> File content/browser/renderer_host/pepper/pepper_file_io_host.cc
> (right):
>
>
https://codereview.chromium.org/33053002/diff/230001/content/browser/renderer...
> content/browser/renderer_host/pepper/pepper_file_io_host.cc:434:
> browser_ppapi_host_->GetDocumentURLForInstance(pp_instance()))) {
> On 2013/10/23 16:54:27, teravest wrote:
>>
>> On 2013/10/23 07:16:04, kinuko wrote:
>> > Instead of returning raw render_process_host ptr from UI to IO
>
> thread (which I
>>
>> > don't feel quite right) can we call this on UI thread and simply
>
> return the
>>
>> > boolean result value to IO thread?
>
>
>> The problem is that BrowserPpapiHost lives entirely on the I/O thread.
>> I suppose I could get the BrowserContext on the UI thread instead of
>
> the
>>
>> RenderProcessHost, but I'm not sure that's much better.
>
>
> I see. Could we do this then: call
> BrowserPpapiHost::GetDocumentURLForInstance() on the IO thread first,
> pass the document URL to the UI thread to check if it's allowed and
> return back the result to IO thread.

Sounds good, done.

>
> As you wrote below RPH doesn't have explicit thread restrictions on some
> methods, but I think our usual assumption is: when it's not explicitly
> noted that it's thread-safe it tends not to be so. (You can take a look
> at RPH code and how most of methods are implemented without thread
> restrictions but in a non-thread-safe way)  GetBrowserContext() looks
> safe, but we need to make double sure that PepperFileIOHost never
> outlives RPH and GetBrowserContext keeps to be thread-safe.  I'd like to
> avoid adding such implicit assumptions there.

Fair enough. Let me know if this looks sane as is. What's the right
way to tighten up the documentation on RenderProcessHost? I'm happy to
add some comments and DCHECK()s there if that's the right thing to do.

>
> https://codereview.chromium.org/33053002/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[kinuko, 2013-10-24 15:37:32]

lgtm (for the files I reviewed), thanks!

> > As you wrote below RPH doesn't have explicit thread restrictions on some
> > methods, but I think our usual assumption is: when it's not explicitly
> > noted that it's thread-safe it tends not to be so. (You can take a look
> > at RPH code and how most of methods are implemented without thread
> > restrictions but in a non-thread-safe way)  GetBrowserContext() looks
> > safe, but we need to make double sure that PepperFileIOHost never
> > outlives RPH and GetBrowserContext keeps to be thread-safe.  I'd like to
> > avoid adding such implicit assumptions there.
> 
> Fair enough. Let me know if this looks sane as is. What's the right
> way to tighten up the documentation on RenderProcessHost? I'm happy to
> add some comments and DCHECK()s there if that's the right thing to do.

Yes, I think adding comments and DCHECKs will be surely helpful.

[commit-bot: I haz the power, 2013-10-24 15:46:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/teravest@chromium.org/33053002/570001

[commit-bot: I haz the power, 2013-10-24 21:20:55]

Retried try job too often on win_rel for step(s) nacl_integration
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-10-24 22:34:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/teravest@chromium.org/33053002/570001

[commit-bot: I haz the power, 2013-10-25 02:51:07]

Retried try job too often on win_rel for step(s) nacl_integration
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[teravest, 2013-10-25 17:19:34]

I (think) was able to debug what's going wrong here-- the (int) "render process
id" on windows isn't the same as the actual process ID that should be sent to
BrokerGetFileHandleForProcess().

It'll take a little while to fix since it means more playing around with the UI
thread, but not more than a day or two.

[commit-bot: I haz the power, 2013-10-30 15:45:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/teravest@chromium.org/33053002/1020001

[commit-bot: I haz the power, 2013-10-30 19:37:25]

Retried try job too often on win7_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[commit-bot: I haz the power, 2013-10-30 19:53:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/teravest@chromium.org/33053002/1020001

[commit-bot: I haz the power, 2013-10-30 22:35:46]

Failed to trigger a try job on win7_aura
HTTP Error 400: Bad Request

[commit-bot: I haz the power, 2013-10-31 13:57:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/teravest@chromium.org/33053002/1240001

[commit-bot: I haz the power, 2013-10-31 16:53:28]

Retried try job too often on linux_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-10-31 16:58:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/teravest@chromium.org/33053002/1240001

[commit-bot: I haz the power, 2013-10-31 19:44:14]

Retried try job too often on linux_chromeos for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2013-10-31 19:51:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/teravest@chromium.org/33053002/1240001

[commit-bot: I haz the power, 2013-10-31 23:12:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/teravest@chromium.org/33053002/1240001

[commit-bot: I haz the power, 2013-11-01 01:33:50]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[teravest, 2013-11-01 14:32:44]

Disabled a (buggy) newly introduced test which caused problems with this change:
  https://codereview.chromium.org/55823004/

[commit-bot: I haz the power, 2013-11-01 14:33:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/teravest@chromium.org/33053002/1240001

[commit-bot: I haz the power, 2013-11-01 19:33:10]

Change committed as 232440

[csharp1, 2013-11-01 20:37:19]

On 2013/11/01 19:33:10, I haz the power (commit-bot) wrote:
> Change committed as 232440

Reverted change because it caused the Linux ASAN bot to break.
http://build.chromium.org/p/chromium.memory/builders/Linux%20ASAN%20Tests%20%...

Failure:
PPAPINaClGLibcTest.FileIO: 
Did not complete.

[teravest, 2013-11-01 21:48:41]

Yep, that was a legitimate ASAN failure.

[commit-bot: I haz the power, 2013-11-01 22:09:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/teravest@chromium.org/33053002/2010001

[commit-bot: I haz the power, 2013-11-02 01:39:47]

Change committed as 232547

[commit-bot: I haz the power, 2013-11-13 16:12:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/teravest@chromium.org/33053002/2310001

[commit-bot: I haz the power, 2013-11-13 20:09:27]

Change committed as 234883