Note that 0 is a valid value for a file descriptor, but some logic in the files app must treat it as an invalid value.

The result is that the file descriptor is leaked and since open created a lock on the server/file share you will be unable to delete the file until the lock is released.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/5dac37aec06c518cc7a5ea138150c57e736d079e

commit 5dac37aec06c518cc7a5ea138150c57e736d079e
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Tue Dec 11 05:47:38 2018

smbprovider: Change the IdMap to set initial ID value in its constructor

- IdMap will now initialize its initial ID value via its constructor.
- Fixes bug that prevents a Close File from occuring due to the 0-value
  file descriptor.
- Updated unit tests + test helpers to reflect on changes to IdMap.

BUG= chromium:909974 
TEST=end-to-end + unit tests

Change-Id: I25d50ba465344ba255f927e76eb18a6c79830e5a
Reviewed-on: https://chromium-review.googlesource.com/1354189
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/5dac37aec06c518cc7a5ea138150c57e736d079e/smbprovider/smbprovider_test.cc
[modify] https://crrev.com/5dac37aec06c518cc7a5ea138150c57e736d079e/smbprovider/samba_interface_impl.cc
[modify] https://crrev.com/5dac37aec06c518cc7a5ea138150c57e736d079e/smbprovider/mount_tracker.cc
[modify] https://crrev.com/5dac37aec06c518cc7a5ea138150c57e736d079e/smbprovider/smbprovider.cc
[modify] https://crrev.com/5dac37aec06c518cc7a5ea138150c57e736d079e/smbprovider/id_map.h
[modify] https://crrev.com/5dac37aec06c518cc7a5ea138150c57e736d079e/smbprovider/id_map_test.cc
[modify] https://crrev.com/5dac37aec06c518cc7a5ea138150c57e736d079e/smbprovider/fake_samba_interface.cc
[modify] https://crrev.com/5dac37aec06c518cc7a5ea138150c57e736d079e/smbprovider/constants.h
[modify] https://crrev.com/5dac37aec06c518cc7a5ea138150c57e736d079e/smbprovider/fake_samba_test.cc
[modify] https://crrev.com/5dac37aec06c518cc7a5ea138150c57e736d079e/smbprovider/fake_samba_interface.h
[modify] https://crrev.com/5dac37aec06c518cc7a5ea138150c57e736d079e/smbprovider/smbprovider_helper_test.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/98d6dae496c0e71bf2b6f132dc56d9ed9f74d5ce

commit 98d6dae496c0e71bf2b6f132dc56d9ed9f74d5ce
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Tue Dec 11 05:47:39 2018

smbprovider: Strengthen Dchecks to reflect on File Descriptor IdMap's initial value change

- IdMap's initial value for file descriptors has been changed from 0
  to 1.
- This change only strengthens Dchecks for file descriptors as previous
  Dchecks would still pass even though changes were made to the intial
  value of file descriptors.

BUG= chromium:909974 
TEST=end-to-end

Change-Id: If34b88cbacf8f9b3c7e7433906d0829da98cceec
Reviewed-on: https://chromium-review.googlesource.com/1356102
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/98d6dae496c0e71bf2b6f132dc56d9ed9f74d5ce/smbprovider/samba_interface_impl.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/897dcb686f7f753a43d4f5a44248aa28c14b704c

commit 897dcb686f7f753a43d4f5a44248aa28c14b704c
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Thu Dec 13 01:04:57 2018

smbprovider: Close file descriptors in SambaInterfaceImpl's destructor

- During the destruction of SambaInterfaceImpl, all opened file
  descriptors will be closed.
- The file descriptor IdMap will also reset during SambaInterfaceImpl's
  destruction.
- If there are any leftover file descriptors prior to the call of
  SambaInterfaceImpl's destruction, it will log the number of leftover
  file descriptors.
- Added unit tests to IdMap to reflect on adding reset + clear
  functions.

BUG= chromium:909974 
TEST=end-to-end

Change-Id: Ie2e656e0966420d08b7e4f23d8fb7a6450395efa
Reviewed-on: https://chromium-review.googlesource.com/1355821
Commit-Ready: jimmy gong <jimmyxgong@chromium.org>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/897dcb686f7f753a43d4f5a44248aa28c14b704c/smbprovider/samba_interface_impl.h
[modify] https://crrev.com/897dcb686f7f753a43d4f5a44248aa28c14b704c/smbprovider/samba_interface_impl.cc
[modify] https://crrev.com/897dcb686f7f753a43d4f5a44248aa28c14b704c/smbprovider/id_map.h
[modify] https://crrev.com/897dcb686f7f753a43d4f5a44248aa28c14b704c/smbprovider/id_map_test.cc

Verified fixed in M-73 with the following steps:

- Create an empty directory
- Copy then paste a file into that directory
- Copy and paste a second file to the at directory
- Delete the first file

The file deleted successfully.

Chrome OS: 11435.0.0, Chrome: 73.0.3644.0, device: Nautilus

Does it need to be merged in M-72?

Only the CL in #4 needs to be merged.

This bug requires manual review: M72 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot