New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 796334 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

AddWatcher called even if watchable is set to false in implementation of ProviderInterface

Project Member Reported by allenvic@chromium.org, Dec 19 2017

Issue description


What steps will reproduce the problem?
(1) Start  SmbClient with watchable set to false. https://cs.chromium.org/chromium/src/chrome/browser/chromeos/smb_client/smb_provider.cc?sq=package:chromium&dr=CSs&l=18
(2) Add logs to SmbFileSystem#AddWatcher
https://cs.chromium.org/chromium/src/chrome/browser/chromeos/smb_client/smb_file_system.cc?sq=package:chromium&dr=CSs&l=218
(3) Mount an SMB and go through the directory on the files app.
(4) You will notice that logs are being called even though the methods shouldn't be called

What is the expected result?
AddWatcher should not be called on a Provider that has watchable set to false.

What happens instead?
AddWater (and RemoveWatcher) is called.

 
Looks like a bug. We always install the watcher even though the file system is not watchable. For now you can just ignore those calls.
Summary: AddWatcher called even if watchable is set to false in implementation of ProviderInterface (was: AddWatcher called even if watchable is set to true in implementation of ProviderInterface)
Components: Platform>Apps>FileManager
Owner: mtomasz@chromium.org
Status: Started (was: Untriaged)
Status: Fixed (was: Started)
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 23 2018

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

commit 072d519850dea414398c1e3dcea4c1ed7b21fa6e
Author: Bailey Berro <baileyberro@chromium.org>
Date: Fri Mar 23 17:07:48 2018

clean up SmbFileSystem member variables

This change includes some cleanup, removing unused member variables and
removing a reference to a bug that has been fixed.

Bug:  chromium:757625 ,  chromium:796334 
Change-Id: I6a5901f355050a47aeb59850e34ed463b79b6899
Reviewed-on: https://chromium-review.googlesource.com/978379
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Commit-Queue: Zentaro Kavanagh <zentaro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545494}
[modify] https://crrev.com/072d519850dea414398c1e3dcea4c1ed7b21fa6e/chrome/browser/chromeos/smb_client/smb_file_system.cc
[modify] https://crrev.com/072d519850dea414398c1e3dcea4c1ed7b21fa6e/chrome/browser/chromeos/smb_client/smb_file_system.h

Status: Assigned (was: Fixed)
It seems that GetWatchers is still being called in some places. Specifically: https://cs.chromium.org/chromium/src/chrome/browser/chromeos/file_system_provider/service.cc?dr=CSs&sq=package:chromium&l=185

and 

https://cs.chromium.org/chromium/src/chrome/browser/chromeos/file_system_provider/service.cc?dr=CSs&sq=package:chromium&l=379

I submitted a CL that adds a check to watchable() to address these issues.
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 28 2018

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

commit 67af16ed4b5c8dcca09f8b6dd0d892c4e9e57c93
Author: Allen Vicencio <allenvic@chromium.org>
Date: Wed Mar 28 12:25:12 2018

[FSP] Do not get watchers for unwatchable file systems

Provided file systems which are not watchable shouldn't have its
GetWatchers method called during Mount and RestoreFileSystems.

Test: Unit tests pass and manual test of RestoreFileSystems
Bug:  796334 
Change-Id: Ibc592e20f8f2cb26843117aa31ec5fcfc627f870
Reviewed-on: https://chromium-review.googlesource.com/982646
Reviewed-by: Tomasz Mikolajewski <mtomasz@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546461}
[modify] https://crrev.com/67af16ed4b5c8dcca09f8b6dd0d892c4e9e57c93/chrome/browser/chromeos/file_system_provider/service.cc
[modify] https://crrev.com/67af16ed4b5c8dcca09f8b6dd0d892c4e9e57c93/chrome/browser/chromeos/file_system_provider/service_unittest.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Apr 4 2018

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

commit e5c8b23f960e569acf621534dcb478eaa7c3ed62
Author: Bailey Berro <baileyberro@chromium.org>
Date: Wed Apr 04 17:27:10 2018

Do not call GetWatchers() on unwatchable provider

This is a follow up on crrev.com/c/982646

Bug:  796334 
Change-Id: I63b04d2f2ab61ef1d9a45f3d360df5f1e1290959
Reviewed-on: https://chromium-review.googlesource.com/988742
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548111}
[modify] https://crrev.com/e5c8b23f960e569acf621534dcb478eaa7c3ed62/chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_api.cc

Cc: mtomasz@chromium.org
Owner: baileyberro@chromium.org
Status: Verified (was: Assigned)

Sign in to add a comment