New issue
Advanced search Search tips

Issue 896219 link

Starred by 1 user

Issue metadata

Status: Closed
Owner: ----
Closed: Oct 18
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: ----



Sign in to add a comment

browser_tests failing on chromium.memory/Linux ChromiumOS MSan Tests

Project Member Reported by sheriff-...@appspot.gserviceaccount.com, Oct 17

Issue description

Filed by sheriff-o-matic@appspot.gserviceaccount.com on behalf of cfroussios@chromium.org

browser_tests failing on chromium.memory/Linux ChromiumOS MSan Tests
Also webui_polymer2_browser_tests, single_process_mash_browser_tests and viz_browser_tests

Builders failed on: 
- Linux ChromiumOS MSan Tests: 
  https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20ChromiumOS%20MSan%20Tests

Several video-related tests failing
Example failure
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20ChromiumOS%20MSan%20Tests/9115

 
Lots of candidates in the list :/
I will revert CLs speculatively...
The best candidate is https://chromium-review.googlesource.com/c/1278619
FilesApp appears in several error logs

file_manager_browsertest_base.cc also appears in error logs, and the other CLs that touched it is already reverted https://chromium-review.googlesource.com/c/1278624

However, I cannot revert due to merge conflicts.
I will see if they're trivial and attempt to resolve...
Reverting with https://chromium-review.googlesource.com/c/chromium/src/+/1286146
(I will run the tests because of the conflicts)
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/103069350367d5c2cf85897e99e899b77a82f6b8

commit 103069350367d5c2cf85897e99e899b77a82f6b8
Author: Christos Froussios <cfroussios@chromium.org>
Date: Wed Oct 17 13:55:27 2018

Revert "Files app: Select My files when there are no volumes"

This reverts commit 6f65bd26330cf709e4a4c4eb60876de5849f94d5.

Reason for revert: Speculative revert
Suspected of breaking browser_tests, ebui_polymer2_browser_tests, single_process_mash_browser_tests and viz_browser_tests
on Linux ChromiumOS MSan

Note: resolved conflicts with ES6 refactoring, therefore running tests...

Bug:  896219 

Original change's description:
> Files app: Select My files when there are no volumes
>
> Make Files app select "My files" when there are no available volumes,
> this to allow Files app to behave properly when volumes subsequently
> become available.
>
> Change DirectoryModel.onVolumeInfoListUpdated_ method to check for
> non-null |displayRoot| before trying to change to |displayRoot|. This
> fixes the error "Cannot read property 'getParent' of null" when Drive
> volume becomes available before the (default) Downloads volume/root.
>
> Change FakeDriveFs to unbind the two mojo bindings if they're bound, so
> DriveFsTestVolume can re-mount itself.
>
> Changes since revert:
> 1. Add a new function |getVolumesCount| to return the number of volumes
> available in the background page.
> 2. Add the new function above to wait for background page to have all
> volumes before un-mounting and wait it to unmount all volumes.
> 3. Change some logs from error to warn because errors are expected and
> handled when initializing volumes that are unmounting/unmounted in the
> backend.
> 4. Change selector used to click on Drive to actually wait for
> "My Drive" to be available.
> 5. A bit of more info in some logs around volume initialization.
>
> Test: browser_tests --gtest_filter="*/fileDisplayWithoutVolumesThenMount*"
> Bug: 893161,  884967 ,  894799 
> Change-Id: I7dcb340991750e9e79a9963990b614c6d7275890
> Reviewed-on: https://chromium-review.googlesource.com/c/1278619
> Reviewed-by: Noel Gordon <noel@chromium.org>
> Reviewed-by: Sam McNally <sammc@chromium.org>
> Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#600282}

TBR=noel@chromium.org,sammc@chromium.org,lucmult@chromium.org

Change-Id: Ifaadea8b49cffc6deb70b429a42bab39d4a3d52b
Bug: 893161,  884967 ,  894799 
Reviewed-on: https://chromium-review.googlesource.com/c/1286146
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Reviewed-by: Christos Froussios <cfroussios@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600376}
[modify] https://crrev.com/103069350367d5c2cf85897e99e899b77a82f6b8/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
[modify] https://crrev.com/103069350367d5c2cf85897e99e899b77a82f6b8/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/103069350367d5c2cf85897e99e899b77a82f6b8/chromeos/components/drivefs/fake_drivefs.cc
[modify] https://crrev.com/103069350367d5c2cf85897e99e899b77a82f6b8/ui/file_manager/file_manager/background/js/test_util_base.js
[modify] https://crrev.com/103069350367d5c2cf85897e99e899b77a82f6b8/ui/file_manager/file_manager/background/js/volume_manager_impl.js
[modify] https://crrev.com/103069350367d5c2cf85897e99e899b77a82f6b8/ui/file_manager/file_manager/background/js/volume_manager_util.js
[modify] https://crrev.com/103069350367d5c2cf85897e99e899b77a82f6b8/ui/file_manager/file_manager/foreground/js/directory_model.js
[modify] https://crrev.com/103069350367d5c2cf85897e99e899b77a82f6b8/ui/file_manager/file_manager/foreground/js/file_manager.js
[modify] https://crrev.com/103069350367d5c2cf85897e99e899b77a82f6b8/ui/file_manager/integration_tests/file_manager/file_display.js

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/033dff086bc81067ff0a4f5dff54434b5293aa9a

commit 033dff086bc81067ff0a4f5dff54434b5293aa9a
Author: Christos Froussios <cfroussios@chromium.org>
Date: Wed Oct 17 16:52:42 2018

Reland "Files app: Select My files when there are no volumes"

This reverts commit 103069350367d5c2cf85897e99e899b77a82f6b8.

Reason for revert: Undoing speculative revert
The CL was not the culprit

Original change's description:
> Revert "Files app: Select My files when there are no volumes"
> 
> This reverts commit 6f65bd26330cf709e4a4c4eb60876de5849f94d5.
> 
> Reason for revert: Speculative revert
> Suspected of breaking browser_tests, ebui_polymer2_browser_tests, single_process_mash_browser_tests and viz_browser_tests
> on Linux ChromiumOS MSan
> 
> Note: resolved conflicts with ES6 refactoring, therefore running tests...
> 
> Bug:  896219 
> 
> Original change's description:
> > Files app: Select My files when there are no volumes
> >
> > Make Files app select "My files" when there are no available volumes,
> > this to allow Files app to behave properly when volumes subsequently
> > become available.
> >
> > Change DirectoryModel.onVolumeInfoListUpdated_ method to check for
> > non-null |displayRoot| before trying to change to |displayRoot|. This
> > fixes the error "Cannot read property 'getParent' of null" when Drive
> > volume becomes available before the (default) Downloads volume/root.
> >
> > Change FakeDriveFs to unbind the two mojo bindings if they're bound, so
> > DriveFsTestVolume can re-mount itself.
> >
> > Changes since revert:
> > 1. Add a new function |getVolumesCount| to return the number of volumes
> > available in the background page.
> > 2. Add the new function above to wait for background page to have all
> > volumes before un-mounting and wait it to unmount all volumes.
> > 3. Change some logs from error to warn because errors are expected and
> > handled when initializing volumes that are unmounting/unmounted in the
> > backend.
> > 4. Change selector used to click on Drive to actually wait for
> > "My Drive" to be available.
> > 5. A bit of more info in some logs around volume initialization.
> >
> > Test: browser_tests --gtest_filter="*/fileDisplayWithoutVolumesThenMount*"
> > Bug: 893161,  884967 ,  894799 
> > Change-Id: I7dcb340991750e9e79a9963990b614c6d7275890
> > Reviewed-on: https://chromium-review.googlesource.com/c/1278619
> > Reviewed-by: Noel Gordon <noel@chromium.org>
> > Reviewed-by: Sam McNally <sammc@chromium.org>
> > Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#600282}
> 
> TBR=noel@chromium.org,sammc@chromium.org,lucmult@chromium.org
> 
> Change-Id: Ifaadea8b49cffc6deb70b429a42bab39d4a3d52b
> Bug: 893161,  884967 ,  894799 
> Reviewed-on: https://chromium-review.googlesource.com/c/1286146
> Commit-Queue: Christos Froussios <cfroussios@chromium.org>
> Reviewed-by: Christos Froussios <cfroussios@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#600376}

TBR=noel@chromium.org,sammc@chromium.org,cfroussios@chromium.org,lucmult@chromium.org

Change-Id: I388a94fe12c7d40ac7ea779353c3b220bd5b5952
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  896219 , 893161,  884967 ,  894799 
Reviewed-on: https://chromium-review.googlesource.com/c/1286853
Reviewed-by: Christos Froussios <cfroussios@chromium.org>
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600429}
[modify] https://crrev.com/033dff086bc81067ff0a4f5dff54434b5293aa9a/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
[modify] https://crrev.com/033dff086bc81067ff0a4f5dff54434b5293aa9a/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/033dff086bc81067ff0a4f5dff54434b5293aa9a/chromeos/components/drivefs/fake_drivefs.cc
[modify] https://crrev.com/033dff086bc81067ff0a4f5dff54434b5293aa9a/ui/file_manager/file_manager/background/js/test_util_base.js
[modify] https://crrev.com/033dff086bc81067ff0a4f5dff54434b5293aa9a/ui/file_manager/file_manager/background/js/volume_manager_impl.js
[modify] https://crrev.com/033dff086bc81067ff0a4f5dff54434b5293aa9a/ui/file_manager/file_manager/background/js/volume_manager_util.js
[modify] https://crrev.com/033dff086bc81067ff0a4f5dff54434b5293aa9a/ui/file_manager/file_manager/foreground/js/directory_model.js
[modify] https://crrev.com/033dff086bc81067ff0a4f5dff54434b5293aa9a/ui/file_manager/file_manager/foreground/js/file_manager.js
[modify] https://crrev.com/033dff086bc81067ff0a4f5dff54434b5293aa9a/ui/file_manager/integration_tests/file_manager/file_display.js

Status: Closed (was: Available)
the tests stopped failing. Not sure what fixed it but there is a number of video related CLs in there

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 18

Labels: merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/512139f8a5dfba65abc441b55973e1659eafcf87

commit 512139f8a5dfba65abc441b55973e1659eafcf87
Author: Luciano Pacheco <lucmult@chromium.org>
Date: Thu Oct 18 22:49:40 2018

Reland "Files app: Select My files when there are no volumes"

This reverts commit 103069350367d5c2cf85897e99e899b77a82f6b8.

Reason for revert: Undoing speculative revert
The CL was not the culprit

Original change's description:
> Revert "Files app: Select My files when there are no volumes"
>
> This reverts commit 6f65bd26330cf709e4a4c4eb60876de5849f94d5.
>
> Reason for revert: Speculative revert
> Suspected of breaking browser_tests, ebui_polymer2_browser_tests, single_process_mash_browser_tests and viz_browser_tests
> on Linux ChromiumOS MSan
>
> Note: resolved conflicts with ES6 refactoring, therefore running tests...
>
> Bug:  896219 
>
> Original change's description:
> > Files app: Select My files when there are no volumes
> >
> > Make Files app select "My files" when there are no available volumes,
> > this to allow Files app to behave properly when volumes subsequently
> > become available.
> >
> > Change DirectoryModel.onVolumeInfoListUpdated_ method to check for
> > non-null |displayRoot| before trying to change to |displayRoot|. This
> > fixes the error "Cannot read property 'getParent' of null" when Drive
> > volume becomes available before the (default) Downloads volume/root.
> >
> > Change FakeDriveFs to unbind the two mojo bindings if they're bound, so
> > DriveFsTestVolume can re-mount itself.
> >
> > Changes since revert:
> > 1. Add a new function |getVolumesCount| to return the number of volumes
> > available in the background page.
> > 2. Add the new function above to wait for background page to have all
> > volumes before un-mounting and wait it to unmount all volumes.
> > 3. Change some logs from error to warn because errors are expected and
> > handled when initializing volumes that are unmounting/unmounted in the
> > backend.
> > 4. Change selector used to click on Drive to actually wait for
> > "My Drive" to be available.
> > 5. A bit of more info in some logs around volume initialization.
> >
> > Test: browser_tests --gtest_filter="*/fileDisplayWithoutVolumesThenMount*"
> > Bug: 893161,  884967 ,  894799 
> > Change-Id: I7dcb340991750e9e79a9963990b614c6d7275890
> > Reviewed-on: https://chromium-review.googlesource.com/c/1278619
> > Reviewed-by: Noel Gordon <noel@chromium.org>
> > Reviewed-by: Sam McNally <sammc@chromium.org>
> > Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#600282}
>
> TBR=noel@chromium.org,sammc@chromium.org,lucmult@chromium.org
>
> Change-Id: Ifaadea8b49cffc6deb70b429a42bab39d4a3d52b
> Bug: 893161,  884967 ,  894799 
> Reviewed-on: https://chromium-review.googlesource.com/c/1286146
> Commit-Queue: Christos Froussios <cfroussios@chromium.org>
> Reviewed-by: Christos Froussios <cfroussios@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#600376}

TBR=noel@chromium.org,sammc@chromium.org,cfroussios@chromium.org,lucmult@chromium.org

Change-Id: I388a94fe12c7d40ac7ea779353c3b220bd5b5952
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  896219 , 893161,  884967 ,  894799 
Reviewed-on: https://chromium-review.googlesource.com/c/1286853
Reviewed-by: Christos Froussios <cfroussios@chromium.org>
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#600429}
Reviewed-on: https://chromium-review.googlesource.com/c/1287350
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#136}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/512139f8a5dfba65abc441b55973e1659eafcf87/ui/file_manager/file_manager/background/js/test_util_base.js
[modify] https://crrev.com/512139f8a5dfba65abc441b55973e1659eafcf87/ui/file_manager/file_manager/background/js/volume_manager_impl.js
[modify] https://crrev.com/512139f8a5dfba65abc441b55973e1659eafcf87/ui/file_manager/file_manager/background/js/volume_manager_util.js
[modify] https://crrev.com/512139f8a5dfba65abc441b55973e1659eafcf87/ui/file_manager/file_manager/foreground/js/directory_model.js
[modify] https://crrev.com/512139f8a5dfba65abc441b55973e1659eafcf87/ui/file_manager/file_manager/foreground/js/file_manager.js
[modify] https://crrev.com/512139f8a5dfba65abc441b55973e1659eafcf87/ui/file_manager/integration_tests/file_manager/file_display.js

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/512139f8a5dfba65abc441b55973e1659eafcf87

Commit: 512139f8a5dfba65abc441b55973e1659eafcf87
Author: lucmult@chromium.org
Commiter: lucmult@chromium.org
Date: 2018-10-18 22:49:40 +0000 UTC

Reland "Files app: Select My files when there are no volumes"

This reverts commit 103069350367d5c2cf85897e99e899b77a82f6b8.

Reason for revert: Undoing speculative revert
The CL was not the culprit

Original change's description:
> Revert "Files app: Select My files when there are no volumes"
>
> This reverts commit 6f65bd26330cf709e4a4c4eb60876de5849f94d5.
>
> Reason for revert: Speculative revert
> Suspected of breaking browser_tests, ebui_polymer2_browser_tests, single_process_mash_browser_tests and viz_browser_tests
> on Linux ChromiumOS MSan
>
> Note: resolved conflicts with ES6 refactoring, therefore running tests...
>
> Bug:  896219 
>
> Original change's description:
> > Files app: Select My files when there are no volumes
> >
> > Make Files app select "My files" when there are no available volumes,
> > this to allow Files app to behave properly when volumes subsequently
> > become available.
> >
> > Change DirectoryModel.onVolumeInfoListUpdated_ method to check for
> > non-null |displayRoot| before trying to change to |displayRoot|. This
> > fixes the error "Cannot read property 'getParent' of null" when Drive
> > volume becomes available before the (default) Downloads volume/root.
> >
> > Change FakeDriveFs to unbind the two mojo bindings if they're bound, so
> > DriveFsTestVolume can re-mount itself.
> >
> > Changes since revert:
> > 1. Add a new function |getVolumesCount| to return the number of volumes
> > available in the background page.
> > 2. Add the new function above to wait for background page to have all
> > volumes before un-mounting and wait it to unmount all volumes.
> > 3. Change some logs from error to warn because errors are expected and
> > handled when initializing volumes that are unmounting/unmounted in the
> > backend.
> > 4. Change selector used to click on Drive to actually wait for
> > "My Drive" to be available.
> > 5. A bit of more info in some logs around volume initialization.
> >
> > Test: browser_tests --gtest_filter="*/fileDisplayWithoutVolumesThenMount*"
> > Bug: 893161,  884967 ,  894799 
> > Change-Id: I7dcb340991750e9e79a9963990b614c6d7275890
> > Reviewed-on: https://chromium-review.googlesource.com/c/1278619
> > Reviewed-by: Noel Gordon <noel@chromium.org>
> > Reviewed-by: Sam McNally <sammc@chromium.org>
> > Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#600282}
>
> TBR=noel@chromium.org,sammc@chromium.org,lucmult@chromium.org
>
> Change-Id: Ifaadea8b49cffc6deb70b429a42bab39d4a3d52b
> Bug: 893161,  884967 ,  894799 
> Reviewed-on: https://chromium-review.googlesource.com/c/1286146
> Commit-Queue: Christos Froussios <cfroussios@chromium.org>
> Reviewed-by: Christos Froussios <cfroussios@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#600376}

TBR=noel@chromium.org,sammc@chromium.org,cfroussios@chromium.org,lucmult@chromium.org

Change-Id: I388a94fe12c7d40ac7ea779353c3b220bd5b5952
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  896219 , 893161,  884967 ,  894799 
Reviewed-on: https://chromium-review.googlesource.com/c/1286853
Reviewed-by: Christos Froussios <cfroussios@chromium.org>
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#600429}
Reviewed-on: https://chromium-review.googlesource.com/c/1287350
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#136}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

Sign in to add a comment