Add an unittest for background/volume_manager.js

Add an unittest for background/volume_manager.js

BUG=315436
TEST=out/Release/browser_tests --gtest_filter=FileManagerJsTest.*

Committed: https://crrev.com/592b47ebda6a572d7ee00cd579530553713f2c8c
Cr-Commit-Position: refs/heads/master@{#302230}

[yawano, 2014-10-24 06:50:10]

yawano@chromium.org changed reviewers:
+ hirono@chromium.org

[yawano, 2014-10-24 06:50:11]

Added test cases for background/volume_manager.js. PTAL. Thanks.

[hirono, 2014-10-24 07:12:22]

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
File chrome/test/data/file_manager/unit_tests/mocks/mock_entry.js (right):

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
chrome/test/data/file_manager/unit_tests/mocks/mock_entry.js:49: get
fileSystem() {
Is it needed? We already have filesystem property.

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
File chrome/test/data/file_manager/unit_tests/mocks/mock_file_system.js (right):

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
chrome/test/data/file_manager/unit_tests/mocks/mock_file_system.js:10: function
MockFileSystem(volumeId) {
Could you reuse TestFileSystem?

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/test/data/f...

It looks the MockFileSystem is almost same.

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
File chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js
(right):

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:108:
if(!initialized) {
Is this check is needed?

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:114: *
Returns a sample profile.
Please add @return

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:116:
function getSampleProfile() {
The word "sample" does not sound familiar in our code base.
Could you rename it with getMockProfile()?

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:135:
}).catch (function(error) {
We usually don't add a space after 'catch' if it is a function.

[yawano, 2014-10-27 00:25:08]

Thank you for the review. Removed unnecessary property and added comments. PTAL.

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
File chrome/test/data/file_manager/unit_tests/mocks/mock_entry.js (right):

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
chrome/test/data/file_manager/unit_tests/mocks/mock_entry.js:49: get
fileSystem() {
On 2014/10/24 07:12:21, hirono wrote:
> Is it needed? We already have filesystem property.

Sorry, this method was unnecessary. I deleted.

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
File chrome/test/data/file_manager/unit_tests/mocks/mock_file_system.js (right):

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
chrome/test/data/file_manager/unit_tests/mocks/mock_file_system.js:10: function
MockFileSystem(volumeId) {
On 2014/10/24 07:12:22, hirono wrote:
> Could you reuse TestFileSystem?
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/test/data/f...
> 
> It looks the MockFileSystem is almost same.

While they are very similar, IMHO, they should be different classes since they
mocks different models.

TestFileSystem mocks a file system which is used around
chrome.fileSystemProvider
(https://developer.chrome.com/apps/fileSystemProvider).
On the other hand, MockFileSystem mocks a file system which is returned from
chrome.fileManagerPrivate.

WDYT?

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
File chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js
(right):

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:108:
if(!initialized) {
On 2014/10/24 07:12:22, hirono wrote:
> Is this check is needed?

Yes, if we initialize twice or more, event handlers which are registered by the
initializer of VolumeManager will be removed. Since VolumeManager uses Singleton
model, they are not re-registered by VolumeManager.getInstance of the next test
case.

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:114: *
Returns a sample profile.
On 2014/10/24 07:12:22, hirono wrote:
> Please add @return

Done.

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:116:
function getSampleProfile() {
On 2014/10/24 07:12:22, hirono wrote:
> The word "sample" does not sound familiar in our code base.
> Could you rename it with getMockProfile()?

Done.

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:135:
}).catch (function(error) {
On 2014/10/24 07:12:22, hirono wrote:
> We usually don't add a space after 'catch' if it is a function.

Done.

[hirono, 2014-10-27 02:15:28]

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
File chrome/test/data/file_manager/unit_tests/mocks/mock_file_system.js (right):

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
chrome/test/data/file_manager/unit_tests/mocks/mock_file_system.js:10: function
MockFileSystem(volumeId) {
On 2014/10/27 00:25:08, yawano wrote:
> On 2014/10/24 07:12:22, hirono wrote:
> > Could you reuse TestFileSystem?
> > 
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/test/data/f...
> > 
> > It looks the MockFileSystem is almost same.
> 
> While they are very similar, IMHO, they should be different classes since they
> mocks different models.
> 
> TestFileSystem mocks a file system which is used around
> chrome.fileSystemProvider
> (https://developer.chrome.com/apps/fileSystemProvider).
> On the other hand, MockFileSystem mocks a file system which is returned from
> chrome.fileManagerPrivate.
> 
> WDYT?

Actually I added TestFileSystem class for testing FileOperationManager.
So I don't mind to use the class for testing Files.app stuff.
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/test/data/f...

[yawano, 2014-10-27 05:09:10]

Merged TestFileSystem and MockFileSystem into one class. PTAL. Thanks.

[hirono, 2014-10-27 09:50:12]

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
File chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js
(right):

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:108:
if(!initialized) {
On 2014/10/27 00:25:08, yawano wrote:
> On 2014/10/24 07:12:22, hirono wrote:
> > Is this check is needed?
> 
> Yes, if we initialize twice or more, event handlers which are registered by
the
> initializer of VolumeManager will be removed. Since VolumeManager uses
Singleton
> model, they are not re-registered by VolumeManager.getInstance of the next
test
> case.

You are right.
I'd like to have VolumeManager.revokeInstanceForTesting method in setUp rather
than stopping initialization, for making each test fail independently.
WDYT?

https://codereview.chromium.org/663513003/diff/140001/chrome/test/data/file_m...
File chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js
(right):

https://codereview.chromium.org/663513003/diff/140001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:148:
callback(false);
Could you use 'reportPromise' utility function,
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/test/data/f...

Or add comments such as callback(/* has_error */ false) callback(/* has_error */
true)?

callback(false) looks like a failure.

[yawano, 2014-10-29 04:07:24]

Thank you for the review, and sorry for my late response. Added
VolumeManager.revokeInstanceForTesting and changed to use reportPromise. I also
removed VolumeManager.getInstanceQueue_ since it seems not to be used in the
current code.

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
File chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js
(right):

https://codereview.chromium.org/663513003/diff/40001/chrome/test/data/file_ma...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:108:
if(!initialized) {
On 2014/10/27 09:50:12, hirono wrote:
> On 2014/10/27 00:25:08, yawano wrote:
> > On 2014/10/24 07:12:22, hirono wrote:
> > > Is this check is needed?
> > 
> > Yes, if we initialize twice or more, event handlers which are registered by
> the
> > initializer of VolumeManager will be removed. Since VolumeManager uses
> Singleton
> > model, they are not re-registered by VolumeManager.getInstance of the next
> test
> > case.
> 
> You are right.
> I'd like to have VolumeManager.revokeInstanceForTesting method in setUp rather
> than stopping initialization, for making each test fail independently.
> WDYT?

I agree with you in order to keep independency between test cases.
I added revokeInstanceForTesting in VolumeManager.

https://codereview.chromium.org/663513003/diff/140001/chrome/test/data/file_m...
File chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js
(right):

https://codereview.chromium.org/663513003/diff/140001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:148:
callback(false);
On 2014/10/27 09:50:12, hirono wrote:
> Could you use 'reportPromise' utility function,
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/test/data/f...
> 
> Or add comments such as callback(/* has_error */ false) callback(/* has_error
*/
> true)?
> 
> callback(false) looks like a failure.

Done.

[hirono, 2014-10-29 10:02:46]

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
File chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js
(right):

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:13: * test
failed.
4 spaces are needed when wrapping a line.

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:15: function
reportPromise(promise, callback) {
Could you please add test_util.js and share the util function with other tests?

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:26:
loadTimeData.data = {
Maybe we can just put it at global scope.

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:90: set
driveConnectionState (state) {
We don't add a space between property name and arguments. Same with function.
e.g. function foo() {}

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:158: return
new Promise(function(resolve, reject) {
The Promise is needed?

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:174: return
new Promise(function(resolve, reject) {
The Promise is needed?

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:206:
reportPromise(VolumeManager.getInstance().then(function(volumeManager_) {
volumeManager_ -> volumeManager

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:213:
'filesystem:chrome-extension://extensionid/external/Downloads-test/' +
4 spaces indent.

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:218:
{eventType: 'mount',
We usually do either:

xxxxxxxxxxxxxxxx({
  eventType: x // two spaces indent
});

OR

xxxxxxxxxxxxxxxx(
    {
      eventType: ''
    },
    moreArgument);

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:249: return
new Promise(function(resolve, reject) {
The Promise is needed?

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:264: return
new Promise(function(resolve, reject) {
The Promise is needed?

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:265: var
entry1 = new MockFileEntry(createMockFileSystem('download:Downloads'),
I prefer the specific name such as downloadEntry rathan than numbering.

https://codereview.chromium.org/663513003/diff/160001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/background/js/volume_manager.js (right):

https://codereview.chromium.org/663513003/diff/160001/ui/file_manager/file_ma...
ui/file_manager/file_manager/background/js/volume_manager.js:551: * Revokes an
instance for testing.
very nit: "an" -> "the"
Maybe "the singleton" is more better.

[yawano, 2014-10-30 09:11:47]

Thank you for the review. PTAL.

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
File chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js
(right):

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:13: * test
failed.
On 2014/10/29 10:02:46, hirono wrote:
> 4 spaces are needed when wrapping a line.

Done.

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:15: function
reportPromise(promise, callback) {
On 2014/10/29 10:02:46, hirono wrote:
> Could you please add test_util.js and share the util function with other
tests?

Done.

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:26:
loadTimeData.data = {
On 2014/10/29 10:02:46, hirono wrote:
> Maybe we can just put it at global scope.

Done.

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:90: set
driveConnectionState (state) {
On 2014/10/29 10:02:46, hirono wrote:
> We don't add a space between property name and arguments. Same with function.
> e.g. function foo() {}

Done.

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:158: return
new Promise(function(resolve, reject) {
On 2014/10/29 10:02:46, hirono wrote:
> The Promise is needed?

Done. The promise was unnecessary. I didn't know that when it returns anything
that is not promise, following promise is fulfilled with it.

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:174: return
new Promise(function(resolve, reject) {
On 2014/10/29 10:02:46, hirono wrote:
> The Promise is needed?

Done.

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:206:
reportPromise(VolumeManager.getInstance().then(function(volumeManager_) {
On 2014/10/29 10:02:46, hirono wrote:
> volumeManager_ -> volumeManager

Done. I moved scopes of volumeManager and numberOfVolumes inside this function.

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:213:
'filesystem:chrome-extension://extensionid/external/Downloads-test/' +
On 2014/10/29 10:02:46, hirono wrote:
> 4 spaces indent.

Done.

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:218:
{eventType: 'mount',
On 2014/10/29 10:02:46, hirono wrote:
> We usually do either:
> 
> xxxxxxxxxxxxxxxx({
>   eventType: x // two spaces indent
> });
> 
> OR
> 
> xxxxxxxxxxxxxxxx(
>     {
>       eventType: ''
>     },
>     moreArgument);

Done.

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:249: return
new Promise(function(resolve, reject) {
On 2014/10/29 10:02:46, hirono wrote:
> The Promise is needed?

Done.

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:264: return
new Promise(function(resolve, reject) {
On 2014/10/29 10:02:46, hirono wrote:
> The Promise is needed?

Done.

https://codereview.chromium.org/663513003/diff/160001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:265: var
entry1 = new MockFileEntry(createMockFileSystem('download:Downloads'),
On 2014/10/29 10:02:46, hirono wrote:
> I prefer the specific name such as downloadEntry rathan than numbering.

Done.

https://codereview.chromium.org/663513003/diff/160001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/background/js/volume_manager.js (right):

https://codereview.chromium.org/663513003/diff/160001/ui/file_manager/file_ma...
ui/file_manager/file_manager/background/js/volume_manager.js:551: * Revokes an
instance for testing.
On 2014/10/29 10:02:46, hirono wrote:
> very nit: "an" -> "the"
> Maybe "the singleton" is more better.

Done.

[hirono, 2014-10-30 09:35:51]

lgtm with nits. Thanks!

https://codereview.chromium.org/663513003/diff/200001/chrome/test/data/file_m...
File chrome/test/data/file_manager/unit_tests/test_util.js (right):

https://codereview.chromium.org/663513003/diff/200001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/test_util.js:21: 
nit: The last line is not needed.

https://codereview.chromium.org/663513003/diff/200001/chrome/test/data/file_m...
File chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js
(right):

https://codereview.chromium.org/663513003/diff/200001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:171: return
(new Promise(function(resolve, reject) {
nit: The '()' is not needed.

[yawano, 2014-10-30 10:18:30]

Thank you for the review! I fixed them.

https://codereview.chromium.org/663513003/diff/200001/chrome/test/data/file_m...
File chrome/test/data/file_manager/unit_tests/test_util.js (right):

https://codereview.chromium.org/663513003/diff/200001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/test_util.js:21: 
On 2014/10/30 09:35:50, hirono wrote:
> nit: The last line is not needed.

Done.

https://codereview.chromium.org/663513003/diff/200001/chrome/test/data/file_m...
File chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js
(right):

https://codereview.chromium.org/663513003/diff/200001/chrome/test/data/file_m...
chrome/test/data/file_manager/unit_tests/volume_manager_unittest.js:171: return
(new Promise(function(resolve, reject) {
On 2014/10/30 09:35:50, hirono wrote:
> nit: The '()' is not needed.

Done.

[yawano, 2014-10-30 10:18:35]

The CQ bit was checked by yawano@chromium.org

[commit-bot: I haz the power, 2014-10-30 10:19:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/663513003/220001

[commit-bot: I haz the power, 2014-10-30 10:23:07]

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

[commit-bot: I haz the power, 2014-10-30 10:23:08]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[yawano, 2014-10-31 00:33:40]

The CQ bit was checked by yawano@chromium.org

[commit-bot: I haz the power, 2014-10-31 00:34:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/663513003/220001

[commit-bot: I haz the power, 2014-10-31 00:40:26]

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

[commit-bot: I haz the power, 2014-10-31 00:40:28]

Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_rel_ng on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[yawano, 2014-10-31 01:59:11]

Since we renamed TestFileSystem to MockFileSystem, it broke
import_history_unittest, and I fixed import_history_unittest to pass the test.
PTAL. Thanks!

[hirono, 2014-10-31 02:29:59]

still lgtm

[yawano, 2014-10-31 02:30:35]

The CQ bit was checked by yawano@chromium.org

[commit-bot: I haz the power, 2014-10-31 02:31:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/663513003/260001

[commit-bot: I haz the power, 2014-10-31 02:35:45]

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

[commit-bot: I haz the power, 2014-10-31 02:35:46]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[yawano, 2014-10-31 03:51:48]

The CQ bit was checked by yawano@chromium.org

[commit-bot: I haz the power, 2014-10-31 03:52:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/663513003/260001

[commit-bot: I haz the power, 2014-10-31 03:56:14]

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

[commit-bot: I haz the power, 2014-10-31 03:56:15]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[yawano, 2014-10-31 06:00:57]

The CQ bit was checked by yawano@chromium.org

[commit-bot: I haz the power, 2014-10-31 06:01:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/663513003/260001

[commit-bot: I haz the power, 2014-10-31 06:05:04]

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

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

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[yawano, 2014-10-31 06:11:00]

yawano@chromium.org changed reviewers:
+ mtomasz@chromium.org

[yawano, 2014-10-31 06:11:01]

@mtomasz: PTAL this CL? Thank you.

[mtomasz, 2014-10-31 06:13:54]

On 2014/10/31 06:11:01, yawano wrote:
> @mtomasz: PTAL this CL? Thank you.

file_manager_jstest.cc lgtm

[yawano, 2014-10-31 06:15:11]

On 2014/10/31 06:13:54, mtomasz wrote:
> On 2014/10/31 06:11:01, yawano wrote:
> > @mtomasz: PTAL this CL? Thank you.
> 
> file_manager_jstest.cc lgtm

Thank you!

[yawano, 2014-10-31 06:15:21]

The CQ bit was checked by yawano@chromium.org

[commit-bot: I haz the power, 2014-10-31 06:16:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/663513003/260001

[commit-bot: I haz the power, 2014-10-31 06:18:28]

Committed patchset #14 (id:260001)

[commit-bot: I haz the power, 2014-10-31 06:19:07]

Patchset 14 (id:??) landed as
https://crrev.com/592b47ebda6a572d7ee00cd579530553713f2c8c
Cr-Commit-Position: refs/heads/master@{#302230}