shell/Windows: Avoid memory allocations and copies when querying icons

IsMemberOf is called for every file (in the ownCloud directory or not) and
with every instance of OCOverlay (we have 5) when displaying a list of
files in Explorer.

Refactor the code to avoid copying the list of watched directories, as
well as creating a wstring from a PWWSTR for files outside watched
directories.

Also change some calls of begin_with to use isDescendantOf since it
properly handles parent paths not ending with a backslash, which could
lead to SocketAPI queries for sibling folders with a name that starts with
a watched folder name.

shell_integration/windows/OCContextMenu/OCContextMenu.cpp

@@ -152,8 +152,9 @@ IFACEMETHODIMP OCContextMenu::QueryContextMenu(HMENU hMenu, UINT indexMenu, UINT
 
     OCClientInterface::ContextMenuInfo info = OCClientInterface::FetchInfo();
     bool skip = true;
+    size_t selectedFileLength = wcslen(m_szSelectedFile);
     for (const std::wstring path : info.watchedDirectories) {
-        if (StringUtil::begins_with(std::wstring(m_szSelectedFile), path)) {
+        if (StringUtil::isDescendantOf(m_szSelectedFile, selectedFileLength, path)) {
             skip = false;
             break;
         }

shell_integration/windows/OCOverlays/OCOverlay.cpp

@@ -126,17 +126,19 @@ IFACEMETHODIMP OCOverlay::GetPriority(int *pPriority)
     return S_OK;
 }
 
- IFACEMETHODIMP OCOverlay::IsMemberOf(PCWSTR pwszPath, DWORD dwAttrib)
+IFACEMETHODIMP OCOverlay::IsMemberOf(PCWSTR pwszPath, DWORD dwAttrib)
 {
     RemotePathChecker* checker = getGlobalChecker();
-    auto watchedDirectories = checker->WatchedDirectories();
+    std::shared_ptr<const std::vector<std::wstring>> watchedDirectories = checker->WatchedDirectories();
+
+    if (watchedDirectories->empty()) {
+        return MAKE_HRESULT(S_FALSE, 0, 0);
+    }
 
-    wstring wpath(pwszPath);
-    wpath.append(L"\\");
-    vector<wstring>::iterator it;
     bool watched = false;
-    for (it = watchedDirectories.begin(); it != watchedDirectories.end(); ++it) {
-        if (StringUtil::begins_with(wpath, *it)) {
+    size_t pathLength = wcslen(pwszPath);
+    for (auto it = watchedDirectories->begin(); it != watchedDirectories->end(); ++it) {
+        if (StringUtil::isDescendantOf(pwszPath, pathLength, *it)) {
             watched = true;
         }
     }
@@ -166,20 +168,3 @@ IFACEMETHODIMP OCOverlay::GetOverlayInfo(PWSTR pwszIconFile, int cchMax, int *pI
 
     return S_OK;
 }
-
-
-bool OCOverlay::_IsOverlaysEnabled()
-{
-    //int enable;
-    bool success = false;
-
-    //if(RegistryUtil::ReadRegistry(REGISTRY_ROOT_KEY, REGISTRY_ENABLE_OVERLAY, &enable))
-    //{
-    //  if(enable) {
-    //      success = true;
-    //  }
-    //}
-
-    return success;
-}
-

shell_integration/windows/OCOverlays/OCOverlay.h

@@ -34,7 +34,6 @@ protected:
     ~OCOverlay();
 
 private:
-    bool _IsOverlaysEnabled();
     long _referenceCount;
     int _state;
 };

shell_integration/windows/OCUtil/RemotePathChecker.cpp

@@ -75,25 +75,30 @@ void RemotePathChecker::workerThreadLoop()
             if (StringUtil::begins_with(response, wstring(L"REGISTER_PATH:"))) {
                 wstring responsePath = response.substr(14); // length of REGISTER_PATH:
 
-                {   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.
+                auto sharedPtrCopy = atomic_load(&_watchedDirectories);
+                auto vectorCopy = make_shared<vector<wstring>>(*sharedPtrCopy);
+                vectorCopy->push_back(responsePath);
+                atomic_store(&_watchedDirectories, shared_ptr<const vector<wstring>>(vectorCopy));
+
+                // 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 so that any shortcut to the root
+                // is updated without the user needing to refresh.
                 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:
 
+                auto sharedPtrCopy = atomic_load(&_watchedDirectories);
+                auto vectorCopy = make_shared<vector<wstring>>(*sharedPtrCopy);
+                vectorCopy->erase(
+                    std::remove(vectorCopy->begin(), vectorCopy->end(), responsePath),
+                    vectorCopy->end());
+                atomic_store(&_watchedDirectories, shared_ptr<const vector<wstring>>(vectorCopy));
+
                 vector<wstring> removedPaths;
                 {   std::unique_lock<std::mutex> lock(_mutex);
-                    _watchedDirectories.erase(
-                        std::remove(_watchedDirectories.begin(), _watchedDirectories.end(), responsePath),
-                        _watchedDirectories.end());
-
                     // Remove any item from the cache
                     for (auto it = _cache.begin(); it != _cache.end() ; ) {
-                        if (StringUtil::begins_with(it->first, responsePath)) {
+                        if (StringUtil::isDescendantOf(it->first, responsePath)) {
                             removedPaths.emplace_back(move(it->first));
                             it = _cache.erase(it);
                         } else {
@@ -143,8 +148,8 @@ void RemotePathChecker::workerThreadLoop()
         }
 
         if (socket.Event() == INVALID_HANDLE_VALUE) {
+            atomic_store(&_watchedDirectories, make_shared<const vector<wstring>>());
             std::unique_lock<std::mutex> lock(_mutex);
-            _watchedDirectories.clear();
             _connected = connected = false;
 
             // Swap to make a copy of the cache under the mutex and clear the one stored.
@@ -167,7 +172,8 @@ void RemotePathChecker::workerThreadLoop()
 
 
 RemotePathChecker::RemotePathChecker()
-    : _connected(false)
+    : _watchedDirectories(make_shared<const vector<wstring>>())
+    , _connected(false)
     , _newQueries(CreateEvent(NULL, true, true, NULL))
     , _thread([this]{ this->workerThreadLoop(); })
 {
@@ -182,10 +188,9 @@ RemotePathChecker::~RemotePathChecker()
     CloseHandle(_newQueries);
 }
 
-vector<wstring> RemotePathChecker::WatchedDirectories()
+std::shared_ptr<const std::vector<std::wstring>> RemotePathChecker::WatchedDirectories() const
 {
-    std::unique_lock<std::mutex> lock(_mutex);
-    return _watchedDirectories;
+    return atomic_load(&_watchedDirectories);
 }
 
 bool RemotePathChecker::IsMonitoredPath(const wchar_t* filePath, int* state)

shell_integration/windows/OCUtil/RemotePathChecker.h

@@ -19,6 +19,7 @@
 #include <unordered_map>
 #include <queue>
 #include <thread>
+#include <memory>
 #include <mutex>
 #include <atomic>
 #include <condition_variable>
@@ -37,7 +38,7 @@ public:
     };
     RemotePathChecker();
     ~RemotePathChecker();
-    std::vector<std::wstring> WatchedDirectories();
+    std::shared_ptr<const std::vector<std::wstring>> WatchedDirectories() const;
     bool IsMonitoredPath(const wchar_t* filePath, int* state);
 
 private:
@@ -52,7 +53,9 @@ private:
     std::queue<std::wstring> _pending;
 
     std::unordered_map<std::wstring, FileState> _cache;
-    std::vector<std::wstring> _watchedDirectories;
+    // The vector is const since it will be accessed from multiple threads through OCOverlay::IsMemberOf.
+    // Each modification needs to be made onto a copy and then atomically replaced in the shared_ptr.
+    std::shared_ptr<const std::vector<std::wstring>> _watchedDirectories;
     bool _connected;
 
 

shell_integration/windows/OCUtil/StringUtil.h

@@ -29,6 +29,21 @@ public:
         return input.size() >= match.size()
             && std::equal(match.begin(), match.end(), input.begin());
     }
+
+    static bool isDescendantOf(const std::wstring& child, const std::wstring& parent) {
+        return isDescendantOf(child.c_str(), child.size(), parent.c_str(), parent.size());
+    }
+
+    static bool isDescendantOf(PCWSTR child, size_t childLength, const std::wstring& parent) {
+        return isDescendantOf(child, childLength, parent.c_str(), parent.size());
+    }
+
+    static bool isDescendantOf(PCWSTR child, size_t childLength, PCWSTR parent, size_t parentLength) {
+        if (!parentLength)
+            return false;
+        return (childLength == parentLength || childLength > parentLength && (child[parentLength] == L'\\' || child[parentLength - 1] == L'\\'))
+            && wcsncmp(child, parent, parentLength) == 0;
+    }
 };
 
 #endif // STRINGUTIL_H