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.

6 files changed, 52 insertions, 44 deletions

diff --git a/shell_integration/windows/OCContextMenu/OCContextMenu.cpp b/shell_integration/windows/OCContextMenu/OCContextMenu.cpp
index 65ff203e1..a5558fc0a 100644
--- a/shell_integration/windows/OCContextMenu/OCContextMenu.cpp
+++ b/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;
         }
diff --git a/shell_integration/windows/OCOverlays/OCOverlay.cpp b/shell_integration/windows/OCOverlays/OCOverlay.cpp
index 1a07205bf..dd03a8664 100644
--- a/shell_integration/windows/OCOverlays/OCOverlay.cpp
+++ b/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;
-}
-
diff --git a/shell_integration/windows/OCOverlays/OCOverlay.h b/shell_integration/windows/OCOverlays/OCOverlay.h
index fc8bc4104..af1aaf696 100644
--- a/shell_integration/windows/OCOverlays/OCOverlay.h
+++ b/shell_integration/windows/OCOverlays/OCOverlay.h
@@ -34,7 +34,6 @@ protected:
     ~OCOverlay();
 
 private:
-    bool _IsOverlaysEnabled();
     long _referenceCount;
     int _state;
 };
diff --git a/shell_integration/windows/OCUtil/RemotePathChecker.cpp b/shell_integration/windows/OCUtil/RemotePathChecker.cpp
index 679686ec7..cf8b72bbd 100644
--- a/shell_integration/windows/OCUtil/RemotePathChecker.cpp
+++ b/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)
diff --git a/shell_integration/windows/OCUtil/RemotePathChecker.h b/shell_integration/windows/OCUtil/RemotePathChecker.h
index e85629f9a..36bcf1ae9 100644
--- a/shell_integration/windows/OCUtil/RemotePathChecker.h
+++ b/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;
 
 
diff --git a/shell_integration/windows/OCUtil/StringUtil.h b/shell_integration/windows/OCUtil/StringUtil.h
index 8614639ac..0f52d51dd 100644
--- a/shell_integration/windows/OCUtil/StringUtil.h
+++ b/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