Sniff MIME type for files which have unknown extensions.

Sniff MIME type for files which have unknown extensions.

What I want to do:
Open text files which have unknown extensions(e.g. access.log.1) using packaged apps which can handle 'text/plain'(e.g. Text, Caret,...).

Current situation:
By following reasons, text files with unknow extension can't be opened.
1. FileManager guess the MIME type as empty, so 'text/plain'-supporting apps are not shown as handlers.
2. PlatformAppLauncher guess the MIME type as 'application/octet-stream', and it launch apps with no data because those apps don't handle 'application/octet-stream'.

What I changed:
Modified FileManager and PlatformAppLauncher to sniff MIME types if they are unknown based on extensions.

BUG=352250
R=benwells@chromium.org, hashimoto@chromium.org, jorgelo@chromium.org

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

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

[fukino, 2014-04-10 08:47:31]

hashimoto san,
If you have time, could you take a look at File Manager part in advance?

[hashimoto, 2014-04-10 09:29:11]

looking good!

https://codereview.chromium.org/224883008/diff/10001/apps/launcher.cc
File apps/launcher.cc (right):

https://codereview.chromium.org/224883008/diff/10001/apps/launcher.cc#newcode195
apps/launcher.cc:195: std::string(),
nit: How about adding a comment like "// type_hit" to tell future readers what
this string is?

https://codereview.chromium.org/224883008/diff/10001/chrome/browser/chromeos/...
File chrome/browser/chromeos/extensions/file_manager/private_api_tasks.cc
(right):

https://codereview.chromium.org/224883008/diff/10001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/private_api_tasks.cc:166:
BrowserThread::PostBlockingPoolTask(
You can use PostBlockingPoolTaskAndReply instead of manually posting the reply.

The code should look like:

PathAndMimeTypeSet* path_mime_set_ptr = path_mime_set.get();
std::vector<GURL>* file_urls_ptr = file_urls.get();

BrowserThread::PostBlockingPoolTask(
  FROM_HERE,
  base::Bind(&StartSniffingMimeType, path_mime_set_ptr, file_urls_ptr),
  base::Bind(&OnSniffingMimeTypeCompleted, base::Passed(&path_mime_set),
base::Passed(&file_urls));

Note: path_mime_set_ptr and file_urls_ptr are necessary because base::Passed may
set NULL to path_mime_set and file_urls.
C++ spec does not define the order of evaluation of function arguments.

https://codereview.chromium.org/224883008/diff/10001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/private_api_tasks.cc:175: void
FileBrowserPrivateGetFileTasksFunction::StartSniffingMimeType(
Since this function is not accessing any member of the class, I think this can
be a free function in the unnamed namespace.

https://codereview.chromium.org/224883008/diff/10001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/private_api_tasks.cc:178:
DCHECK(!BrowserThread::CurrentlyOn(content::BrowserThread::UI));
nit: We usually don't have this kind of DCHECK.
File IO functions like base::ReadFile is already performing
ThreadRestrictions::AssertIOAllowed() in them.

https://codereview.chromium.org/224883008/diff/10001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/private_api_tasks.cc:180:
scoped_ptr<PathAndMimeTypeSet> sniffed_path_mime_set(new PathAndMimeTypeSet);
nit: Why don't you modify path_mime_set directly?

https://codereview.chromium.org/224883008/diff/10001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/private_api_tasks.cc:183: // For
each files, sniff its MIME type if it is empty
nit: Please add a note about that we sniff local files only.

[fukino, 2014-04-10 11:12:05]

Thank you for comments!

https://codereview.chromium.org/224883008/diff/10001/apps/launcher.cc
File apps/launcher.cc (right):

https://codereview.chromium.org/224883008/diff/10001/apps/launcher.cc#newcode195
apps/launcher.cc:195: std::string(),
On 2014/04/10 09:29:12, hashimoto wrote:
> nit: How about adding a comment like "// type_hit" to tell future readers what
> this string is?

Done.

https://codereview.chromium.org/224883008/diff/10001/chrome/browser/chromeos/...
File chrome/browser/chromeos/extensions/file_manager/private_api_tasks.cc
(right):

https://codereview.chromium.org/224883008/diff/10001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/private_api_tasks.cc:166:
BrowserThread::PostBlockingPoolTask(
On 2014/04/10 09:29:12, hashimoto wrote:
> You can use PostBlockingPoolTaskAndReply instead of manually posting the
reply.
> 
> The code should look like:
> 
> PathAndMimeTypeSet* path_mime_set_ptr = path_mime_set.get();
> std::vector<GURL>* file_urls_ptr = file_urls.get();
> 
> BrowserThread::PostBlockingPoolTask(
>   FROM_HERE,
>   base::Bind(&StartSniffingMimeType, path_mime_set_ptr, file_urls_ptr),
>   base::Bind(&OnSniffingMimeTypeCompleted, base::Passed(&path_mime_set),
> base::Passed(&file_urls));
> 
> Note: path_mime_set_ptr and file_urls_ptr are necessary because base::Passed
may
> set NULL to path_mime_set and file_urls.
> C++ spec does not define the order of evaluation of function arguments.

Thank you for sample code and note!
I would be caught in the trap without your advice.

https://codereview.chromium.org/224883008/diff/10001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/private_api_tasks.cc:175: void
FileBrowserPrivateGetFileTasksFunction::StartSniffingMimeType(
On 2014/04/10 09:29:12, hashimoto wrote:
> Since this function is not accessing any member of the class, I think this can
> be a free function in the unnamed namespace.

Done.

https://codereview.chromium.org/224883008/diff/10001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/private_api_tasks.cc:178:
DCHECK(!BrowserThread::CurrentlyOn(content::BrowserThread::UI));
On 2014/04/10 09:29:12, hashimoto wrote:
> nit: We usually don't have this kind of DCHECK.
> File IO functions like base::ReadFile is already performing
> ThreadRestrictions::AssertIOAllowed() in them.

Deleted DCHECK.

https://codereview.chromium.org/224883008/diff/10001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/private_api_tasks.cc:180:
scoped_ptr<PathAndMimeTypeSet> sniffed_path_mime_set(new PathAndMimeTypeSet);
On 2014/04/10 09:29:12, hashimoto wrote:
> nit: Why don't you modify path_mime_set directly?

path_mime_set is std::set, so I avoid modifying keys in the iteration.
(Modifying only mime types won't break the container under condition that the
file paths are unique, though)
(And I don't know why the PathAndMimeSet is set...)

https://codereview.chromium.org/224883008/diff/10001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/private_api_tasks.cc:183: // For
each files, sniff its MIME type if it is empty
On 2014/04/10 09:29:12, hashimoto wrote:
> nit: Please add a note about that we sniff local files only.

Done.

[hashimoto, 2014-04-10 11:54:21]

lgtm with a nit

https://codereview.chromium.org/224883008/diff/10001/chrome/browser/chromeos/...
File chrome/browser/chromeos/extensions/file_manager/private_api_tasks.cc
(right):

https://codereview.chromium.org/224883008/diff/10001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/private_api_tasks.cc:180:
scoped_ptr<PathAndMimeTypeSet> sniffed_path_mime_set(new PathAndMimeTypeSet);
On 2014/04/10 11:12:06, fukino wrote:
> On 2014/04/10 09:29:12, hashimoto wrote:
> > nit: Why don't you modify path_mime_set directly?
> 
> path_mime_set is std::set, so I avoid modifying keys in the iteration.
> (Modifying only mime types won't break the container under condition that the
> file paths are unique, though)
> (And I don't know why the PathAndMimeSet is set...)

Ah, that makes sense.
Thank you for clarification.

https://codereview.chromium.org/224883008/diff/40004/chrome/browser/chromeos/...
File chrome/browser/chromeos/extensions/file_manager/private_api_tasks.cc
(right):

https://codereview.chromium.org/224883008/diff/40004/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/private_api_tasks.cc:74: //
content via network frequently.
This comment is misleading as we can get MIME type of the file from the Drive
server so that there is no need to sniff Drive files (of course, its cost is
also the reason why we don't do it.)

[fukino, 2014-04-10 12:10:58]

https://codereview.chromium.org/224883008/diff/40004/chrome/browser/chromeos/...
File chrome/browser/chromeos/extensions/file_manager/private_api_tasks.cc
(right):

https://codereview.chromium.org/224883008/diff/40004/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/private_api_tasks.cc:74: //
content via network frequently.
On 2014/04/10 11:54:21, hashimoto wrote:
> This comment is misleading as we can get MIME type of the file from the Drive
> server so that there is no need to sniff Drive files (of course, its cost is
> also the reason why we don't do it.)

Done.

[fukino, 2014-04-10 12:14:39]

Hi Ben,
could you take a look at apps/launcher.cc?

[fukino, 2014-04-10 15:17:28]

Hi Jorge,

I'm making a minor change about how to launch platform apps with files (diff in
apps/launcher.cc).
Could you take a look from the viewpoint of security?

Before: 
  When the file's MIME type is not known besed on its extention,
PlatformAppLancher launch the app without passing the file.
After :
  When the file's MIME type is not known based on its extension,
PlatformAppLauncher sniff its MIME type by reading file's content (up to 1KB).
  If sniffed MIME type, which can be wrong type, is compatible with the app,
PlatformAppLauncher launches the app with those files.

[Jorge Lucangeli Obes, 2014-04-10 20:36:08]

https://codereview.chromium.org/224883008/diff/60001/apps/launcher.cc
File apps/launcher.cc (right):

https://codereview.chromium.org/224883008/diff/60001/apps/launcher.cc#newcode191
apps/launcher.cc:191: if (bytes_read >= 0) {
Why do we want to sniff if bytes_read == 0?

[fukino, 2014-04-10 23:50:32]

Thanks for comment!

https://codereview.chromium.org/224883008/diff/60001/apps/launcher.cc
File apps/launcher.cc (right):

https://codereview.chromium.org/224883008/diff/60001/apps/launcher.cc#newcode191
apps/launcher.cc:191: if (bytes_read >= 0) {
On 2014/04/10 20:36:08, Jorge Lucangeli Obes wrote:
> Why do we want to sniff if bytes_read == 0?

When we call sniffMimeFile() to empty file, its MIME type is guessed as
'text/plain'.
I think it is sometimes convenient for users to be able to open empty files by
text editor apps.

[Jorge Lucangeli Obes, 2014-04-10 23:53:40]

On 2014/04/10 23:50:32, fukino wrote:
> Thanks for comment!
> 
> https://codereview.chromium.org/224883008/diff/60001/apps/launcher.cc
> File apps/launcher.cc (right):
> 
>
https://codereview.chromium.org/224883008/diff/60001/apps/launcher.cc#newcode191
> apps/launcher.cc:191: if (bytes_read >= 0) {
> On 2014/04/10 20:36:08, Jorge Lucangeli Obes wrote:
> > Why do we want to sniff if bytes_read == 0?
> 
> When we call sniffMimeFile() to empty file, its MIME type is guessed as
> 'text/plain'.
> I think it is sometimes convenient for users to be able to open empty files by
> text editor apps.

Yeah, that sounds very reasonable.

Are we opening this MIME-sniffed files in Chrome apps only? I'm trying to
understand the impact of an intentionally-malicious file designed to fool the
MIME-sniffing algorithm to be opened in one of our apps.

[fukino, 2014-04-11 00:32:01]

On 2014/04/10 23:53:40, Jorge Lucangeli Obes wrote:
> On 2014/04/10 23:50:32, fukino wrote:
> > Thanks for comment!
> > 
> > https://codereview.chromium.org/224883008/diff/60001/apps/launcher.cc
> > File apps/launcher.cc (right):
> > 
> >
>
https://codereview.chromium.org/224883008/diff/60001/apps/launcher.cc#newcode191
> > apps/launcher.cc:191: if (bytes_read >= 0) {
> > On 2014/04/10 20:36:08, Jorge Lucangeli Obes wrote:
> > > Why do we want to sniff if bytes_read == 0?
> > 
> > When we call sniffMimeFile() to empty file, its MIME type is guessed as
> > 'text/plain'.
> > I think it is sometimes convenient for users to be able to open empty files
by
> > text editor apps.
> 
> Yeah, that sounds very reasonable.
> 
> Are we opening this MIME-sniffed files in Chrome apps only? I'm trying to
> understand the impact of an intentionally-malicious file designed to fool the
> MIME-sniffing algorithm to be opened in one of our apps.

I think so. We are opening MIME-sniffed files in Chrome apps only.
Ben, could you let me know if my understanding is not correct?

Regarding malicious files, it may be similar situation with that the malicious
user change file's extension intentionally to be opened in one of our apps.
The sniffing algorithm itself doesn't seem harmful for me because it only read
the content and categorize them by byte patterns and byte occurrences
distributions.
https://code.google.com/p/chromium/codesearch#chromium/src/net/base/mime_snif...

[Jorge Lucangeli Obes, 2014-04-11 18:38:59]

On 2014/04/11 00:32:01, fukino wrote:
> On 2014/04/10 23:53:40, Jorge Lucangeli Obes wrote:
> > On 2014/04/10 23:50:32, fukino wrote:
> > > Thanks for comment!
> > > 
> > > https://codereview.chromium.org/224883008/diff/60001/apps/launcher.cc
> > > File apps/launcher.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/224883008/diff/60001/apps/launcher.cc#newcode191
> > > apps/launcher.cc:191: if (bytes_read >= 0) {
> > > On 2014/04/10 20:36:08, Jorge Lucangeli Obes wrote:
> > > > Why do we want to sniff if bytes_read == 0?
> > > 
> > > When we call sniffMimeFile() to empty file, its MIME type is guessed as
> > > 'text/plain'.
> > > I think it is sometimes convenient for users to be able to open empty
files
> by
> > > text editor apps.
> > 
> > Yeah, that sounds very reasonable.
> > 
> > Are we opening this MIME-sniffed files in Chrome apps only? I'm trying to
> > understand the impact of an intentionally-malicious file designed to fool
the
> > MIME-sniffing algorithm to be opened in one of our apps.
> 
> I think so. We are opening MIME-sniffed files in Chrome apps only.
> Ben, could you let me know if my understanding is not correct?
> 

This sounds good, waiting on a confirmation by Ben.

> Regarding malicious files, it may be similar situation with that the malicious
> user change file's extension intentionally to be opened in one of our apps.
> The sniffing algorithm itself doesn't seem harmful for me because it only read
> the content and categorize them by byte patterns and byte occurrences
> distributions.
>
https://code.google.com/p/chromium/codesearch#chromium/src/net/base/mime_snif...

[benwells, 2014-04-11 23:22:34]

https://codereview.chromium.org/224883008/diff/60001/apps/launcher.cc
File apps/launcher.cc (right):

https://codereview.chromium.org/224883008/diff/60001/apps/launcher.cc#newcode189
apps/launcher.cc:189: std::vector<char> content(net::kMaxBytesToSniff);
Is there a reason not to use char buf[net::kMaxBytesToSniff]?

[fukino, 2014-04-12 06:21:14]

https://codereview.chromium.org/224883008/diff/60001/apps/launcher.cc
File apps/launcher.cc (right):

https://codereview.chromium.org/224883008/diff/60001/apps/launcher.cc#newcode189
apps/launcher.cc:189: std::vector<char> content(net::kMaxBytesToSniff);
On 2014/04/11 23:22:35, benwells wrote:
> Is there a reason not to use char buf[net::kMaxBytesToSniff]?

I thought net::kMaxBytesToSniff was somewhat big and might be bigger in the
future, so I avoided allocating buffer on the stack.

[benwells, 2014-04-14 05:47:41]

lgtm.

just to confirm, the apps/launcher.cc code is for launching chrome packaged
apps. In the future it might also be enabled for more types of chrome app like
things, such as component extensions like quickoffice or the pdf viewer.

https://codereview.chromium.org/224883008/diff/60001/apps/launcher.cc
File apps/launcher.cc (right):

https://codereview.chromium.org/224883008/diff/60001/apps/launcher.cc#newcode189
apps/launcher.cc:189: std::vector<char> content(net::kMaxBytesToSniff);
On 2014/04/12 06:21:15, fukino wrote:
> On 2014/04/11 23:22:35, benwells wrote:
> > Is there a reason not to use char buf[net::kMaxBytesToSniff]?
> 
> I thought net::kMaxBytesToSniff was somewhat big and might be bigger in the
> future, so I avoided allocating buffer on the stack.

ok, fair enough.

[fukino, 2014-04-14 07:05:04]

On 2014/04/14 05:47:41, benwells wrote:
> lgtm.
> 
> just to confirm, the apps/launcher.cc code is for launching chrome packaged
> apps. In the future it might also be enabled for more types of chrome app like
> things, such as component extensions like quickoffice or the pdf viewer.
> 
> https://codereview.chromium.org/224883008/diff/60001/apps/launcher.cc
> File apps/launcher.cc (right):
> 
>
https://codereview.chromium.org/224883008/diff/60001/apps/launcher.cc#newcode189
> apps/launcher.cc:189: std::vector<char> content(net::kMaxBytesToSniff);
> On 2014/04/12 06:21:15, fukino wrote:
> > On 2014/04/11 23:22:35, benwells wrote:
> > > Is there a reason not to use char buf[net::kMaxBytesToSniff]?
> > 
> > I thought net::kMaxBytesToSniff was somewhat big and might be bigger in the
> > future, so I avoided allocating buffer on the stack.
> 
> ok, fair enough.

> just to confirm, the apps/launcher.cc code is for launching chrome packaged
> apps. In the future it might also be enabled for more types of chrome app like
> things, such as component extensions like quickoffice or the pdf viewer.

Thank you confirmation!

[Jorge Lucangeli Obes, 2014-04-14 18:56:49]

On 2014/04/14 07:05:04, fukino wrote:
> On 2014/04/14 05:47:41, benwells wrote:
> > lgtm.
> > 
> > just to confirm, the apps/launcher.cc code is for launching chrome packaged
> > apps. In the future it might also be enabled for more types of chrome app
like
> > things, such as component extensions like quickoffice or the pdf viewer.
> > 
> > https://codereview.chromium.org/224883008/diff/60001/apps/launcher.cc
> > File apps/launcher.cc (right):
> > 
> >
>
https://codereview.chromium.org/224883008/diff/60001/apps/launcher.cc#newcode189
> > apps/launcher.cc:189: std::vector<char> content(net::kMaxBytesToSniff);
> > On 2014/04/12 06:21:15, fukino wrote:
> > > On 2014/04/11 23:22:35, benwells wrote:
> > > > Is there a reason not to use char buf[net::kMaxBytesToSniff]?
> > > 
> > > I thought net::kMaxBytesToSniff was somewhat big and might be bigger in
the
> > > future, so I avoided allocating buffer on the stack.
> > 
> > ok, fair enough.
> 
> > just to confirm, the apps/launcher.cc code is for launching chrome packaged
> > apps. In the future it might also be enabled for more types of chrome app
like
> > things, such as component extensions like quickoffice or the pdf viewer.
> 

lgtm. However, whenever we add the above functionality, let's make sure that we
never send MIME-sniffed documents to third-party extensions.

> Thank you confirmation!

[benwells, 2014-04-14 22:05:21]

Any file access to 3rd party extensions will go through a stringent
security review, I'm sure :)


On Tue, Apr 15, 2014 at 4:56 AM, <jorgelo@chromium.org> wrote:

> On 2014/04/14 07:05:04, fukino wrote:
>
>> On 2014/04/14 05:47:41, benwells wrote:
>> > lgtm.
>> >
>> > just to confirm, the apps/launcher.cc code is for launching chrome
>> packaged
>> > apps. In the future it might also be enabled for more types of chrome
>> app
>>
> like
>
>> > things, such as component extensions like quickoffice or the pdf viewer.
>> >
>> > https://codereview.chromium.org/224883008/diff/60001/apps/launcher.cc
>> > File apps/launcher.cc (right):
>> >
>> >
>>
>
> https://codereview.chromium.org/224883008/diff/60001/apps/
> launcher.cc#newcode189
>
>> > apps/launcher.cc:189: std::vector<char> content(net::kMaxBytesToSniff);
>> > On 2014/04/12 06:21:15, fukino wrote:
>> > > On 2014/04/11 23:22:35, benwells wrote:
>> > > > Is there a reason not to use char buf[net::kMaxBytesToSniff]?
>> > >
>> > > I thought net::kMaxBytesToSniff was somewhat big and might be bigger
>> in
>>
> the
>
>> > > future, so I avoided allocating buffer on the stack.
>> >
>> > ok, fair enough.
>>
>
>  > just to confirm, the apps/launcher.cc code is for launching chrome
>> packaged
>> > apps. In the future it might also be enabled for more types of chrome
>> app
>>
> like
>
>> > things, such as component extensions like quickoffice or the pdf viewer.
>>
>
>
> lgtm. However, whenever we add the above functionality, let's make sure
> that we
> never send MIME-sniffed documents to third-party extensions.
>
>
>  Thank you confirmation!
>>
>
>
>
> https://codereview.chromium.org/224883008/
>

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

[fukino, 2014-04-14 22:26:32]

The CQ bit was checked by fukino@chromium.org

[commit-bot: I haz the power, 2014-04-14 22:27:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fukino@chromium.org/224883008/60001

[commit-bot: I haz the power, 2014-04-14 23:31:15]

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

[commit-bot: I haz the power, 2014-04-14 23:31:15]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_rel

[fukino, 2014-04-15 07:28:59]

Hi Ben,
I updated test codes. (Sorry, I forgot to do it.)
Could you take a look?

A new test case covers the case that the file with unknown extension is sniffed
as known MIME type.

https://codereview.chromium.org/224883008/diff/80001/chrome/browser/apps/app_...
File chrome/browser/apps/app_browsertest.cc (right):

https://codereview.chromium.org/224883008/diff/80001/chrome/browser/apps/app_...
chrome/browser/apps/app_browsertest.cc:666:
SetCommandLineArg("platform_apps/launch_files/test_binary.unknownextension");
To force the MIME type to fall back to application/octet-stream,
test_binary.unknowextension is used here. This is a binary file whose content
was generated by random number generator.

[benwells, 2014-04-15 22:15:12]

lgtm

[fukino, 2014-04-15 23:51:43]

On 2014/04/15 22:15:12, benwells wrote:
> lgtm

thanks!

[yoshiki, 2014-04-16 10:02:35]

Committed patchset #6 manually as r264167 (presubmit successful).

[fukino, 2014-04-17 02:00:07]

Ben,
This change was reverted because the newly added test case
(PlatformAppBrowserTest, LaunchWithSniffableType) failed on Win 7 Tests (2).
http://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%282%29/buil...
I'm investigating, and I'll ask you review again after fix. Sorry for bothering
you. 
If you can see the reason, please let me know.

[fukino, 2014-04-17 09:07:58]

The failure with browser_tests on Windows was caused by this issue.
http://crbug.com/243885

I'll try to fix it first.

[benwells, 2014-04-17 09:44:18]

On 2014/04/17 09:07:58, fukino wrote:
> The failure with browser_tests on Windows was caused by this issue.
> http://crbug.com/243885
> 
> I'll try to fix it first.

Thanks fukino, that's awesome.

[fukino, 2014-04-22 10:38:45]

On 2014/04/17 09:44:18, benwells wrote:
> On 2014/04/17 09:07:58, fukino wrote:
> > The failure with browser_tests on Windows was caused by this issue.
> > http://crbug.com/243885
> > 
> > I'll try to fix it first.
> 
> Thanks fukino, that's awesome.

The reason that the previous CL was reverted has been fixed by
https://codereview.chromium.org/240893002/

I'll re-commit this CL.

[fukino, 2014-04-22 10:42:13]

The CQ bit was checked by fukino@chromium.org

[commit-bot: I haz the power, 2014-04-22 10:42:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fukino@chromium.org/224883008/100001

[commit-bot: I haz the power, 2014-04-22 13:01:54]

Change committed as 265239