New issue
Advanced search Search tips

Issue 883571 link

Starred by 1 user
Status: Fixed
Owner:
joelhockey@chromium.org
Closed: Sep 18
Cc:
lucmult@chromium.org
slangley@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

FilesAppBrowserTest.Test/mountCrostiniContainer TypeError: Cannot read property 'volumeType' of null

Project Member Reported by noel@chromium.org, Sep 13 
Interesting logs noticed in FilesAppBrowserTest.Test/mountCrostiniContainer test: null volume type, Mounted volume without a request: crostini:Crostini, etc.

Example:

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/28919

[ RUN      ] Crostini/FilesAppBrowserTest.Test/mountCrostiniContainer
[1652:1652:0912/014900.983802:WARNING:user_policy_manager_factory_chromeos.cc(208)] No policy loaded for known non-enterprise user
[1652:1652:0912/014902.303409:ERROR:gpu_interface_provider.cc(85)] Not implemented reached in virtual void content::GpuInterfaceProvider::RegisterOzoneGpuInterfaces(service_manager::BinderRegistry *)
[1652:1652:0912/014902.486165:WARNING:shelf_button.cc(381)] An icon of size 32x32is being scaled up and will look blurry.
[1652:1652:0912/014903.592885:INFO:file_manager_browsertest_base.cc(1118)] FileManagerBrowserTest::StartTest mountCrostiniContainer
[1652:1652:0912/014911.905670:INFO:CONSOLE(1172)] "Cache database creating or upgrading.", source: chrome-extension://pmfjbimdmchhbnneeidfognadeopoehp/background_scripts.js (1172)
[1652:1652:0912/014912.630742:INFO:CONSOLE(5675)] "Requesting volume list.", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js (5675)
[1652:1652:0912/014912.722325:INFO:CONSOLE(4960)] "Waiting for the result of openMainWindow", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js (4960)
[1652:1652:0912/014912.743734:INFO:CONSOLE(5677)] "Volume list fetched with: 3 items.", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js (5677)
[1652:1652:0912/014912.750052:INFO:CONSOLE(5687)] "Initializing volume: android_files:AndroidFiles", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js (5687)
[1652:1652:0912/014912.758795:INFO:CONSOLE(6081)] "Requesting file system.", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js (6081)
[1652:1652:0912/014912.787363:INFO:CONSOLE(5687)] "Initializing volume: downloads:Downloads", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js (5687)
[1652:1652:0912/014912.791506:INFO:CONSOLE(6081)] "Requesting file system.", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js (6081)
[1652:1652:0912/014912.799953:INFO:CONSOLE(5687)] "Initializing volume: drive:drive-user", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js (5687)
[1652:1652:0912/014912.803946:INFO:CONSOLE(6081)] "Requesting file system.", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js (6081)
[1652:1652:0912/014913.414055:INFO:CONSOLE(6129)] "File system obtained: android_files:AndroidFiles", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js (6129)
[1652:1652:0912/014913.425140:INFO:CONSOLE(5691)] "Initialized volume: android_files:AndroidFiles", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js (5691)
[1652:1652:0912/014913.441525:INFO:CONSOLE(6129)] "File system obtained: downloads:Downloads", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js (6129)
[1652:1652:0912/014913.443284:INFO:CONSOLE(5691)] "Initialized volume: downloads:Downloads", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js (5691)
[1652:1652:0912/014913.567575:INFO:CONSOLE(6129)] "File system obtained: drive:drive-user", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js (6129)
[1652:1652:0912/014913.570459:INFO:CONSOLE(5691)] "Initialized volume: drive:drive-user", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js (5691)
[1652:1652:0912/014913.571089:INFO:CONSOLE(5695)] "Initialized all volumes.", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js (5695)
[1652:1652:0912/014913.889605:INFO:CONSOLE(4957)] "Received the result of openMainWindow", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js (4957)
[1652:1652:0912/014913.905654:INFO:CONSOLE(0)] "HTML Imports is deprecated and will be removed in M73, around March 2019. Please use ES modules instead. See https://www.chromestatus.com/features/5144752345317376 for more details.", source:  (0)
[1652:1652:0912/014914.653475:INFO:CONSOLE(442)] "document.registerElement is deprecated and will be removed in M73, around March 2019. Please use window.customElements.define instead. See https://www.chromestatus.com/features/4642138092470272 for more details.", source: chrome://resources/polymer/v1_0/polymer/polymer-micro-extracted.js (442)
[1652:1652:0912/014918.096726:INFO:CONSOLE(2083)] "Element.createShadowRoot is deprecated and will be removed in M73, around March 2019. Please use Element.attachShadow instead. See https://www.chromestatus.com/features/4507242028072960 for more details.", source: chrome://resources/polymer/v1_0/polymer/polymer-mini-extracted.js (2083)
[1652:1652:0912/014919.741487:INFO:CONSOLE(156)] "at /file_manager/background.js:373:27: The number of file is 0. Not changed.", source: chrome-extension://oobinhbdbiehknkpbpejbbpdbkdjmoco/test_util.js (156)
[1652:1652:0912/014923.117762:INFO:CONSOLE(156)] "at /file_manager/background.js:373:27: The number of file is 0. Not changed.", source: chrome-extension://oobinhbdbiehknkpbpejbbpdbkdjmoco/test_util.js (156)
[1652:1652:0912/014926.283377:INFO:CONSOLE(6081)] "Requesting file system.", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js (6081)
[1652:1652:0912/014927.462864:INFO:CONSOLE(6129)] "File system obtained: crostini:Crostini", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js (6129)
[1652:1652:0912/014929.997835:INFO:CONSOLE(2501)] "Video thumbnail error: ", source: chrome-extension://pmfjbimdmchhbnneeidfognadeopoehp/background_scripts.js (2501)
[1652:1652:0912/014930.045821:INFO:CONSOLE(5754)] "Mounted volume without a request: crostini:Crostini", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js (5754)
[1652:1652:0912/014930.089391:INFO:CONSOLE(0)] "Error in event handler: TypeError: Cannot read property 'volumeType' of null
    at Banners.maybeShowLowSpaceWarning_ (chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/foreground/js/main_scripts.js:41614:18)
    at Banners.privateOnDirectoryChanged_ (chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/foreground/js/main_scripts.js:41603:10)
    at extensions::fileManagerPrivate:243:3", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/main.html (0)
[1652:1652:0912/014930.100659:INFO:CONSOLE(0)] "Error in event handler: TypeError: Cannot read property 'volumeType' of null
    at Banners.maybeShowLowSpaceWarning_ (chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/foreground/js/main_scripts.js:41614:18)
    at Banners.privateOnDirectoryChanged_ (chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/foreground/js/main_scripts.js:41603:10)
    at extensions::fileManagerPrivate:243:3", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/main.html (0)
[1652:1652:0912/014930.110372:INFO:CONSOLE(0)] "Error in event handler: TypeError: Cannot read property 'volumeType' of null
    at Banners.maybeShowLowSpaceWarning_ (chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/foreground/js/main_scripts.js:41614:18)
    at Banners.privateOnDirectoryChanged_ (chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/foreground/js/main_scripts.js:41603:10)
    at extensions::fileManagerPrivate:243:3", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/main.html (0)
[1652:1652:0912/014930.120571:INFO:CONSOLE(0)] "Error in event handler: TypeError: Cannot read property 'volumeType' of null
    at Banners.maybeShowLowSpaceWarning_ (chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/foreground/js/main_scripts.js:41614:18)
    at Banners.privateOnDirectoryChanged_ (chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/foreground/js/main_scripts.js:41603:10)
    at extensions::fileManagerPrivate:243:3", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/main.html (0)
[1652:1652:0912/014930.447690:INFO:CONSOLE(0)] "[SUCCESS] [mountCrostiniContainer]", source: chrome-extension://oobinhbdbiehknkpbpejbbpdbkdjmoco/_generated_background_page.html (0)
[1652:1652:0912/014931.636132:WARNING:event_router.cc(481)] Not all file watchers are removed. This can happen when the Files app is open during shutdown.
[1652:1685:0912/014931.903146:WARNING:discardable_shared_memory_manager.cc(438)] Some MojoDiscardableSharedMemoryManagerImpls are still alive. They will be leaked.
[1652:1652:0912/014931.914037:WARNING:pref_notifier_impl.cc(23)] Pref observer found at shutdown.
[1652:1652:0912/014931.914167:WARNING:pref_notifier_impl.cc(23)] Pref observer found at shutdown.
[       OK ] Crostini/FilesAppBrowserTest.Test/mountCrostiniContainer (33348 ms)

Comment 1 by noel@chromium.org, Sep 13

Labels: OS-Chrome

Comment 2 by slangley@chromium.org, Sep 16

Cc: -joelhockey@chromium.org
Labels: CrOSFilesFeature-Crostini
Owner: joelhockey@chromium.org
Status: Assigned (was: Untriaged)

Comment 3 by lucmult@chromium.org, Sep 17 

This doesn't seem to be an issue with Crostini, but rather an issue with how some parts of our UI aren't taking care of possible null value from:
"getVolumeInfo" and "getLocationInfo".

My understanding these two functions can return null if UI has a reference to an entry from a volume that has been umounted.

This made me think that we should have a lint checker, similar to clang that we could check variables resulting from functions that can return null that don't check before accessing attributes.
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 18 

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

commit 052bac3d5a30345863b012a978a78ad77bfcdc6d
Author: Joel Hockey <joelhockey@chromium.org>
Date: Tue Sep 18 02:12:33 2018

CrOS FilesApp: Fix error messages in integration tests

Mounted volume without a request: crostini:Crostini:
* This is basically what happens when you pull out a USB without
  clicking unmount.  This message is harmless, but the code is changed
  to stop it anyway.
* Message fixed to state 'Unmounted ...' since the error happens at unmount.
* Changed test to call FMP.unmount and added logic into private_api_mount.cc
  to remove crostini mount.
* Removed unmountCrostini test function which is no longer needed.

Error in event handler: TypeError: Cannot read property 'volumeType' of null:
* This error happens when crostini is unmounted and the directory changes
  back to Downloads.  banners.js fixed to guard for the case where the
  volume is now removed and VolumeManager.getLocationInfo returns null.

Bug:  883571 
Change-Id: I13c0bee7f514dfc318eaa12f4c1ef1c2186e7a70
Reviewed-on: https://chromium-review.googlesource.com/1227864
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591916}
[modify] https://crrev.com/052bac3d5a30345863b012a978a78ad77bfcdc6d/chrome/browser/chromeos/extensions/file_manager/private_api_mount.cc
[modify] https://crrev.com/052bac3d5a30345863b012a978a78ad77bfcdc6d/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/052bac3d5a30345863b012a978a78ad77bfcdc6d/ui/file_manager/file_manager/background/js/test_util_base.js
[modify] https://crrev.com/052bac3d5a30345863b012a978a78ad77bfcdc6d/ui/file_manager/file_manager/background/js/volume_manager_impl.js
[modify] https://crrev.com/052bac3d5a30345863b012a978a78ad77bfcdc6d/ui/file_manager/file_manager/foreground/js/ui/banners.js
[modify] https://crrev.com/052bac3d5a30345863b012a978a78ad77bfcdc6d/ui/file_manager/integration_tests/file_manager/crostini.js

Comment 5 by joelhockey@chromium.org, Sep 18

Status: Fixed (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 18 

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

commit 8d138d26a2f0b015be2fe68b1bbdbe0ca557075f
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Tue Sep 18 05:53:34 2018

Revert "CrOS FilesApp: Fix error messages in integration tests"

This reverts commit 052bac3d5a30345863b012a978a78ad77bfcdc6d.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 591916 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzA1MmJhYzNkNWEzMDM0NTg2M2IwMTJhOTc4YTc4YWQ3N2JmY2RjNmQM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.memory/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/29026

Sample Failed Step: browser_tests

Original change's description:
> CrOS FilesApp: Fix error messages in integration tests
> 
> Mounted volume without a request: crostini:Crostini:
> * This is basically what happens when you pull out a USB without
>   clicking unmount.  This message is harmless, but the code is changed
>   to stop it anyway.
> * Message fixed to state 'Unmounted ...' since the error happens at unmount.
> * Changed test to call FMP.unmount and added logic into private_api_mount.cc
>   to remove crostini mount.
> * Removed unmountCrostini test function which is no longer needed.
> 
> Error in event handler: TypeError: Cannot read property 'volumeType' of null:
> * This error happens when crostini is unmounted and the directory changes
>   back to Downloads.  banners.js fixed to guard for the case where the
>   volume is now removed and VolumeManager.getLocationInfo returns null.
> 
> Bug:  883571 
> Change-Id: I13c0bee7f514dfc318eaa12f4c1ef1c2186e7a70
> Reviewed-on: https://chromium-review.googlesource.com/1227864
> Commit-Queue: Joel Hockey <joelhockey@chromium.org>
> Reviewed-by: Noel Gordon <noel@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#591916}

Change-Id: I419e792b050f4a991bfe435907e7fc5b2d45311e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  883571 
Reviewed-on: https://chromium-review.googlesource.com/1229619
Cr-Commit-Position: refs/heads/master@{#591950}
[modify] https://crrev.com/8d138d26a2f0b015be2fe68b1bbdbe0ca557075f/chrome/browser/chromeos/extensions/file_manager/private_api_mount.cc
[modify] https://crrev.com/8d138d26a2f0b015be2fe68b1bbdbe0ca557075f/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/8d138d26a2f0b015be2fe68b1bbdbe0ca557075f/ui/file_manager/file_manager/background/js/test_util_base.js
[modify] https://crrev.com/8d138d26a2f0b015be2fe68b1bbdbe0ca557075f/ui/file_manager/file_manager/background/js/volume_manager_impl.js
[modify] https://crrev.com/8d138d26a2f0b015be2fe68b1bbdbe0ca557075f/ui/file_manager/file_manager/foreground/js/ui/banners.js
[modify] https://crrev.com/8d138d26a2f0b015be2fe68b1bbdbe0ca557075f/ui/file_manager/integration_tests/file_manager/crostini.js
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 18 

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

commit 52144a74b506cf0e896f919d12958ab7ac1cd53a
Author: Joel Hockey <joelhockey@chromium.org>
Date: Tue Sep 18 07:01:28 2018

Reland "CrOS FilesApp: Fix error messages in integration tests"

This is a reland of 052bac3d5a30345863b012a978a78ad77bfcdc6d

ASAN/LSAN now OK.

Changed VolumeManager::RemoveSshfsCrostiniVolume(const FilePath& path) to
call DiskMountManager::UnmountPath(path.value()) before calling
DoUnmountEvent since DoUnmountEvent will destroy the Volume object which
owns the passed in 'path' arg.

Original change's description:
> CrOS FilesApp: Fix error messages in integration tests
>
> Mounted volume without a request: crostini:Crostini:
> * This is basically what happens when you pull out a USB without
>   clicking unmount.  This message is harmless, but the code is changed
>   to stop it anyway.
> * Message fixed to state 'Unmounted ...' since the error happens at unmount.
> * Changed test to call FMP.unmount and added logic into private_api_mount.cc
>   to remove crostini mount.
> * Removed unmountCrostini test function which is no longer needed.
>
> Error in event handler: TypeError: Cannot read property 'volumeType' of null:
> * This error happens when crostini is unmounted and the directory changes
>   back to Downloads.  banners.js fixed to guard for the case where the
>   volume is now removed and VolumeManager.getLocationInfo returns null.
>
> Bug:  883571 
> Change-Id: I13c0bee7f514dfc318eaa12f4c1ef1c2186e7a70
> Reviewed-on: https://chromium-review.googlesource.com/1227864
> Commit-Queue: Joel Hockey <joelhockey@chromium.org>
> Reviewed-by: Noel Gordon <noel@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#591916}

Bug:  883571 
Change-Id: I2e1f23ec71cec9e0b47add912dbe0ee56fbfbc92
Reviewed-on: https://chromium-review.googlesource.com/1229735
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591961}
[modify] https://crrev.com/52144a74b506cf0e896f919d12958ab7ac1cd53a/chrome/browser/chromeos/extensions/file_manager/private_api_mount.cc
[modify] https://crrev.com/52144a74b506cf0e896f919d12958ab7ac1cd53a/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/52144a74b506cf0e896f919d12958ab7ac1cd53a/chrome/browser/chromeos/file_manager/volume_manager.cc
[modify] https://crrev.com/52144a74b506cf0e896f919d12958ab7ac1cd53a/ui/file_manager/file_manager/background/js/test_util_base.js
[modify] https://crrev.com/52144a74b506cf0e896f919d12958ab7ac1cd53a/ui/file_manager/file_manager/background/js/volume_manager_impl.js
[modify] https://crrev.com/52144a74b506cf0e896f919d12958ab7ac1cd53a/ui/file_manager/file_manager/foreground/js/ui/banners.js
[modify] https://crrev.com/52144a74b506cf0e896f919d12958ab7ac1cd53a/ui/file_manager/integration_tests/file_manager/crostini.js
►

Sign in to add a comment