shell/Windows: Remove the UPDATE_VIEW message handling

The client now tries to only push STATUS messages to connections
unless they previously requested it with a RETRIEVE_FILE_STATUS,
but this means that we now have to make sure that a new connection
will rerequest every icon that the user will see on files from that
point on.

To that end, we now send a SHChangeNotify for every file in the cache
when we lose the connection (or on UNREGISTER_PATH) to make sure that
what's on the screen matches what's in the cache. We also remove the
_oldCache logic that made this more difficult to enforce.

The client has been able to reliably push status updates on macOS
for a few versions now, and we don't need it on Windows either. The
_oldCache mechanism was to avoid sending to many update requests
when receiving an UPDATE_VIEW.

Also fixes #4766

shell_integration/windows/OCUtil/RemotePathChecker.cpp

@@ -78,10 +78,14 @@ void RemotePathChecker::workerThreadLoop()
                 {   std::unique_lock<std::mutex> lock(_mutex);
                     _watchedDirectories.push_back(responsePath);
                 }
+                // We don't keep track of all files and can't know which file is currently visible to the user,
+                // but at least reload the root dir + itself so that any shortcut to the root is updated without
+                // the user needing to navigate.
                 SHChangeNotify(SHCNE_UPDATEDIR, SHCNF_PATH | SHCNF_FLUSHNOWAIT, responsePath.data(), NULL);
             } else if (StringUtil::begins_with(response, wstring(L"UNREGISTER_PATH:"))) {
                 wstring responsePath = response.substr(16); // length of UNREGISTER_PATH:
 
+                vector<wstring> removedPaths;
                 {   std::unique_lock<std::mutex> lock(_mutex);
                     _watchedDirectories.erase(
                         std::remove(_watchedDirectories.begin(), _watchedDirectories.end(), responsePath),
@@ -90,15 +94,15 @@ void RemotePathChecker::workerThreadLoop()
                     // Remove any item from the cache
                     for (auto it = _cache.begin(); it != _cache.end() ; ) {
                         if (StringUtil::begins_with(it->first, responsePath)) {
+                            removedPaths.emplace_back(move(it->first));
                             it = _cache.erase(it);
                         } else {
                             ++it;
                         }
                     }
-                    // Assume that we won't need this at this point, UNREGISTER_PATH is rare
-                    _oldCache.clear();
                 }
-                SHChangeNotify(SHCNE_UPDATEDIR, SHCNF_PATH | SHCNF_FLUSHNOWAIT, responsePath.data(), NULL);
+                for (auto& path : removedPaths)
+                    SHChangeNotify(SHCNE_UPDATEITEM, SHCNF_PATH | SHCNF_FLUSHNOWAIT, path.data(), NULL);
             } else if (StringUtil::begins_with(response, wstring(L"STATUS:")) ||
                     StringUtil::begins_with(response, wstring(L"BROADCAST:"))) {
 
@@ -129,29 +133,21 @@ void RemotePathChecker::workerThreadLoop()
                     SHChangeNotify(SHCNE_UPDATEITEM, SHCNF_PATH | SHCNF_FLUSHNOWAIT, responsePath.data(), NULL);
                 }
             }
-            else if (StringUtil::begins_with(response, wstring(L"UPDATE_VIEW"))) {
-                std::unique_lock<std::mutex> lock(_mutex);
-                // Keep the old states to continue having something to display while the new state is
-                // requested from the client, triggered by clearing _cache.
-                _oldCache.insert(_cache.cbegin(), _cache.cend());
-
-                // Swap to make a copy of the cache under the mutex and clear the one stored.
-                std::unordered_map<std::wstring, FileState> cache;
-                swap(cache, _cache);
-                lock.unlock();
-                // Let explorer know about the invalidated cache entries, it will re-request the ones it needs.
-                for (auto it = cache.begin(); it != cache.end(); ++it) {
-                    SHChangeNotify(SHCNE_UPDATEITEM, SHCNF_PATH | SHCNF_FLUSHNOWAIT, it->first.data(), NULL);
-                }
-            }
         }
 
         if (socket.Event() == INVALID_HANDLE_VALUE) {
             std::unique_lock<std::mutex> lock(_mutex);
-            _cache.clear();
-            _oldCache.clear();
             _watchedDirectories.clear();
             _connected = connected = false;
+
+            // Swap to make a copy of the cache under the mutex and clear the one stored.
+            std::unordered_map<std::wstring, FileState> cache;
+            swap(cache, _cache);
+            lock.unlock();
+            // Let explorer know about each invalidated cache entry that needs to get its icon removed.
+            for (auto it = cache.begin(); it != cache.end(); ++it) {
+                SHChangeNotify(SHCNE_UPDATEITEM, SHCNF_PATH | SHCNF_FLUSHNOWAIT, it->first.data(), NULL);
+            }
         }
 
         if (_stop) return;
@@ -198,21 +194,16 @@ bool RemotePathChecker::IsMonitoredPath(const wchar_t* filePath, int* state)
 
     auto it = _cache.find(path);
     if (it != _cache.end()) {
+        // The path is in our cache, and we'll get updates pushed if the status changes.
         *state = it->second;
         return true;
     }
 
-    // Re-request the status while we display what we have in _oldCache
     _pending.push(filePath);
 
-    it = _oldCache.find(path);
-    bool foundInOldCache = it != _oldCache.end();
-    if (foundInOldCache)
-        *state = it->second;
-
     lock.unlock();
     SetEvent(_newQueries);
-    return foundInOldCache;
+    return false;
 }
 
 RemotePathChecker::FileState RemotePathChecker::_StrToFileState(const std::wstring &str)

shell_integration/windows/OCUtil/RemotePathChecker.h

@@ -52,7 +52,6 @@ private:
     std::queue<std::wstring> _pending;
 
     std::unordered_map<std::wstring, FileState> _cache;
-    std::unordered_map<std::wstring, FileState> _oldCache;
     std::vector<std::wstring> _watchedDirectories;
     bool _connected;