Fix: Fixed issues with Google Drive support

[wharvex]

Related Issues

• Bug: Drive is not being recognized #16149

Background

• We don't want Google Drive (or any cloud drive) to appear in the plain "Drives" sections.
• Most of the logic that populates the plain "Drives" sections is in Files.App.Services.StorageDevicesService.GetDrivesAsync().
• In this method, there is a foreach over the drives returned from a basic DriveInfo.GetDrives() call (where each iterated drive is stored in the temporary variable drive).
• At the top of this foreach there is an if that skips the current iteration if the drive is Google Drive or pCloud.
  • (I'll refer to this if as the "Cloud Drive Filter If" -- CDFI)
• Previously, the CDFI compared drive.Name to the file system paths of Google Drive and pCloud.
• To allow this comparison for Google Drive, I got Google Drive's system path from the Registry.
• I got the root filepath (normally G:\) and then the CDFI skipped any drive with that path, under the assumption that this was Google Drive and therefore a cloud drive.
• In hindsight, this logic was not the best. It is possible, as issue Bug: Drive is not being recognized #16149 shows, that a user might have Google Drive listed in their Registry under G:\, but a different, non-cloud drive actually assigned to G:\, which we would want to appear under the plain "Drives" sections.
• The current PR hopefully fixes this issue.

Solution Approach

• Instead of comparing the root Google Drive filepath to drive.Name in the CDFI, use File.App.Utils.Storage.DriveHelpers.GetExtendedDriveLabel to get the DriveInfo.VolumeLabel of the drive, and compare that to the string "Google Drive".
• This approach is better because it filters by an attribute of the drive that only a Google Drive drive would have, and it does not rely on the Registry, which can contain misleading Google Drive artifacts if Google Drive is in a "partially uninstalled" state on the user's system.

Summary of Changes

• StorageDevicesService: Remove logic that (1) gets the Google Drive path with a Registry query, (2) times the Registry query, and (3) saves the Google Drive path to App.AppModel.GoogleDrivePath, because the Google Drive path is no longer needed in StorageDevicesService, it was only used in the CDFI.
• StorageDevicesService: Get the drive label and use it in the CDFI, as described in Solution Approach.
• StorageDevicesService: Edit/add some comments.
• GoogleDriveCloudDetector: Add logic that (1) gets the Google Drive path with a Registry query, (2) times the Registry query (moved back here from StorageDevicesService).
• GoogleDriveCloudDetector: Change a variable name and edit/add some comments.

Test Plans

• Regression tests
  • Test RT1
    • Setup
      • Set Google Drive preferences to Syncing Mirror, StreamLoc DriveLetter
    • Checks
      • Check RT1-C1: Does Google Drive appear under plain "Drives" in sidebar or home?
        • Expected: No
        • Actual: No
      • Check RT1-C2: Does Google Drive appear exactly once under "Cloud Drives"?
        • Expected: Yes
        • Actual: Yes
      • Check RT1-C3: Does clicking on Google Drive open "My Drive" directly?
        • Expected: Yes
        • Actual: Yes
      • Check RT1-C4: Does clicking on Google Drive open the correct file system path for the settings configuration?
        • Expected: Yes
        • Actual: Yes
  • Test RT2
    • Setup
      • Set Google Drive preferences to Syncing Mirror, StreamLoc Folder
    • Checks
      • Check RT2-C1: Does Google Drive appear under plain "Drives" in sidebar or home?
        • Expected: No
        • Actual: No
      • Check RT2-C2: Does Google Drive appear exactly once under "Cloud Drives"?
        • Expected: Yes
        • Actual: Yes
      • Check RT2-C3: Does clicking on Google Drive open "My Drive" directly?
        • Expected: Yes
        • Actual: Yes
      • Check RT2-C4: Does clicking on Google Drive open the correct file system path for the settings configuration?
        • Expected: Yes
        • Actual: Yes
  • Test RT3
    • Setup
      • Set Google Drive preferences to Syncing Stream, StreamLoc Folder
    • Checks
      • Check RT3-C1: Does Google Drive appear under plain "Drives" in sidebar or home?
        • Expected: No
        • Actual: No
      • Check RT3-C2: Does Google Drive appear exactly once under "Cloud Drives"?
        • Expected: Yes
        • Actual: Yes
      • Check RT3-C3: Does clicking on Google Drive open "My Drive" directly?
        • Expected: Yes
        • Actual: Yes
      • Check RT3-C4: Does clicking on Google Drive open the correct file system path for the settings configuration?
        • Expected: Yes
        • Actual: Yes
  • Test RT4
    • Setup
      • Set Google Drive preferences to Syncing Stream, StreamLoc DriveLetter
    • Checks
      • Check RT4-C1: Does Google Drive appear under plain "Drives" in sidebar or home?
        • Expected: No
        • Actual: No
      • Check RT4-C2: Does Google Drive appear exactly once under "Cloud Drives"?
        • Expected: Yes
        • Actual: Yes
      • Check RT4-C3: Does clicking on Google Drive open "My Drive" directly?
        • Expected: Yes
        • Actual: Yes
      • Check RT4-C4: Does clicking on Google Drive open the correct file system path for the settings configuration?
        • Expected: Yes
        • Actual: Yes
• Bugfix tests
  • Test BFT1
    • Summary
      • Confirm that a Google Drive drive has "Google Drive" as its drive label (and is therefore skipped by the CDFI)
    • Setup
      • Set Google Drive preferences to "Syncing set to Stream, Stream Location set to Drive Letter" with drive letter G.
      • Run Files with debugger.
      • Track the driveLabel local variable for each iteration of the foreach in StorageDevicesService.GetDrivesAsync.
    • Checks
      • Check BFT1-C1: Does driveLabel for the drive whose Name is "G:" equal "Google Drive"?
        • Expected: Yes
        • Actual:
  • Test BFT2
    • Summary
      • Confirm that a non-Google Drive drive that has "G:" as its path (which is what KremmenUK's dashcam drive was) does not have "Google Drive" as its drive label (and is therefore not skipped by the CDFI)
    • Setup 1
      • Switch to main branch.
      • Set Google Drive preferences to "Syncing set to Stream, Stream Location set to Drive Letter" with drive letter G.
      • Stop Google Drive process.
      • Plug in a flash drive and set its drive letter to G.
      • Run Files with debugger.
      • Track what GoogleDriveCloudDetector.GetRegistryBasePath finds in the registry.
      • Track the driveLabel local variable for each iteration of the foreach in StorageDevicesService.GetDrivesAsync.
    • Checks 1 (these should show that the changes made to my machine's configuration in Setup 1 reproduce KremmenUK's issue)
      • Check BFT2-C1: Do we see "G" as the mount point?
        • Expected: Yes
        • Actual:
      • Check BFT2-C2: Does the driveLabel for the drive whose Name is "G:" equal "Google Drive"?
        • Expected: No
        • Actual:
      • Check BFT2-C3: Does the flash drive on drive letter G appear in the plain "Drives" sections in the app?
        • Expected: No
        • Actual:
    • Setup 2
      • Switch to BugGoogleDriveNotRecognized branch.
    • Checks 2 (these should show that the changes made to the Files code in this PR branch solve KremmenUK's issue)
      • Check BFT2-C4: Does the flash drive on drive letter G appear in the plain "Drives" sections in the app?
        • Expected: Yes
        • Actual:

[wharvex]

I'll complete the bug fix tests within the next few days and update the PR with the results.

[wharvex]

Thank you @Lamparter for the review! I just committed all the changes and will rebase onto main soon.

[wharvex]

Updated bugfix test plan with the first one completed:

• Bugfix tests
  • Test BFT1
    • Purpose
      • Confirm that a Google Drive drive has "Google Drive" as its drive label.
    • Setup
      • Set Google Drive preferences to "Syncing set to Stream, Stream Location set to Drive Letter" with drive letter G.
      • Run Files with debugger.
      • Track the driveLabel local variable for each iteration of the foreach in StorageDevicesService.GetDrivesAsync().
    • Checks
      • Check BFT1-C1: Does driveLabel for the drive whose Name is G:\ equal "Google Drive"?
        • Expected: Yes
        • Actual: Yes
  • Test BFT2
    • Purpose
      • Reproduce @KremmenUK's issue on the main branch.
    • Setup
      • Switch to main branch in Files repo.
      • Set Google Drive preferences to "Syncing set to Stream, Stream Location set to Drive Letter" with drive letter G.
      • Stop Google Drive process.
      • Plug in a flash drive and set its drive letter to G.
      • Run Files with debugger.
      • Track the googleDriveRegValPropProp local variable in GoogleDriveCloudDetector.GetRegistryBasePath().
      • Track the driveLabel local variable for each iteration of the foreach in StorageDevicesService.GetDrivesAsync().
    • Checks
      • Check BFT2-C1: Does googleDriveRegValPropProp equal "G"?
        • Expected: Yes
        • Actual:
      • Check BFT2-C2: Does the driveLabel for the drive whose Name is G:\ equal "Google Drive"?
        • Expected: No
        • Actual:
      • Check BFT2-C3: Does the flash drive on drive letter G appear in the plain "Drives" sections in the app?
        • Expected: No
        • Actual:
  • Test BFT3
    • Purpose
      • Confirm that the changes made in the bugfix branch fix @KremmenUK's issue.
    • Setup
      • Switch to BugGoogleDriveNotRecognized branch.
      • Set Google Drive preferences to "Syncing set to Stream, Stream Location set to Drive Letter" with drive letter G.
      • Stop Google Drive process.
      • Plug in a flash drive and set its drive letter to G.
      • Run Files.
    • Checks
      • Check BFT3-C1: Does the flash drive on drive letter G appear in the plain "Drives" sections in the app?
        • Expected: Yes
        • Actual:

[wharvex]

Unfortunately I was unable to reproduce @KremmenUK's issue via the method described in my test plan.

My plan was to plug in a USB drive, set its drive letter to G, stop Google Drive while it was on the G:\ mount point, and run Files.

Performing these setup tasks did result in the non-Google-Drive G drive being filtered out by the if-statement in StorageDevicesService which we thought was the culprit.

However, they did not prevent my non-Google-Drive G drive from appearing in the plain "Drives" sections.

So even though that if-statement passed, Files still managed to find my USB Drive and display it, unlike with @KremmenUK's dashcam drive.

Here is a screen recording of the test I did. I will keep trying to figure the issue out, but if anyone has any suggestions please let me know.

[wharvex]

Files.App.Data.Models.DrivesViewModel.Watcher_DeviceAdded() contains a call to DrivesViewModel.Drives.Add(). I think this watcher method is the reason why my non-Google-Drive G drive was still getting added.

[KremmenUK]

Thanks for continuing to look As I replied a while ago, I fixed the problem via one of the suggested fixes which involved removing the link to my obsolete G: [Google drive] and it's not recurred since Pete

…

On Mon, 20 Jan 2025, 02:29 Tim, ***@***.***> wrote: Unfortunately I was unable to reproduce @KremmenUK <https://github.com/KremmenUK>'s issue via the method described in my test plan. My plan was to plug in a USB drive, set its drive letter to G, stop Google Drive while it was on the G:\ mount point, and run Files. Performing these setup tasks *did* result in the non-Google-Drive G drive being filtered out by the if-statement in StorageDevicesService which we thought was the culprit. However, they *did not* prevent my non-Google-Drive G drive from appearing in the plain "Drives" sections. So even though that if-statement passed, Files still managed to find my USB Drive and display it, unlike with @KremmenUK <https://github.com/KremmenUK>'s dashcam drive. Here <https://drive.google.com/file/d/1DCvxHC-tDeIgoUYg97jIpYMaY9eRc7L6/view?usp=sharing> is a screen recording of the test I did. I will keep trying to figure the issue out, but if anyone has any suggestions please let me know. — Reply to this email directly, view it on GitHub <#16706 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BLDQFKCWHFQ25KQU3KTJKBL2LRNP7AVCNFSM6AAAAABVBRGLF6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMBRGE4TMNZTGA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[wharvex]

No problem, glad to hear it!

I have now confirmed that if the call to DrivesViewModel.Drives.Add() in DrivesViewModel.Watcher_DeviceAdded() is skipped (for a non-Google-Drive G drive) when running Files in the main branch with all the rest of the setup in place for reproducing Pete's issue on my machine, the issue does successfully reproduce, and my non-Google-Drive G drive does not appear under plain drives or anywhere in the app.

Here is a screen recording of the test I did to confirm this.

My theory is that for some reason, the DrivesViewModel.watcher.DeviceAdded event was not firing for KremmenUK's drive, but was firing for my drive.

As some evidence for this theory, I searched for and did not find a "Drive added: G" line in the original debug log KremmenUK included with his issue. That line would have appeared in the full log if that DrivesViewModel.watcher.DeviceAdded event had fired for him.

[yaira2]

@wharvex are you ready for a review?

[wharvex]

@yaira2 Yes I think it's ready now.

[Josh65-2201]

I have not been able to reproduce the issue KremmenUK reported but Google Drive is working as it does in Preview 3.8.12.0.
The detection for removing Google Drive from the Drives section is working better on this branch then in Preview too.

[yaira2]

@wharvex thank you