[ros-diffs] [reactos] 01/01: [SHELL32] Optimize change notification (#3030)

[Katayama Hirofumi MZ]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=1f31905ecd4bbf2addf16…

commit 1f31905ecd4bbf2addf1656331d3cf557ff54358
Author:     Katayama Hirofumi MZ <katayama.hirofumi.mz(a)gmail.com>
AuthorDate: Mon Aug 10 13:34:48 2020 +0900
Commit:     GitHub <noreply(a)github.com>
CommitDate: Mon Aug 10 13:34:48 2020 +0900

    [SHELL32] Optimize change notification (#3030)
    
    - Keep the directory lists only.
    - Don't remember file sizes and normal file paths.
    CORE-13950
---
 dll/win32/shell32/shelldesktop/CDirectoryList.cpp  | 122 +++++++++++++
 dll/win32/shell32/shelldesktop/CDirectoryList.h    |  93 ++++++++++
 .../shell32/shelldesktop/CDirectoryWatcher.cpp     |  36 +---
 dll/win32/shell32/shelldesktop/CDirectoryWatcher.h |   4 +-
 dll/win32/shell32/shelldesktop/CFilePathList.cpp   | 192 ---------------------
 dll/win32/shell32/shelldesktop/CFilePathList.h     | 108 ------------
 dll/win32/shell32/shelldesktop/CMakeLists.txt      |   2 +-
 7 files changed, 227 insertions(+), 330 deletions(-)

diff --git a/dll/win32/shell32/shelldesktop/CDirectoryList.cpp
b/dll/win32/shell32/shelldesktop/CDirectoryList.cpp
new file mode 100644
index 00000000000..eff446df467
--- /dev/null
+++ b/dll/win32/shell32/shelldesktop/CDirectoryList.cpp
@@ -0,0 +1,122 @@
+/*
+ * PROJECT:     shell32
+ * LICENSE:     LGPL-2.1-or-later (https://spdx.org/licenses/LGPL-2.1-or-later)
+ * PURPOSE:     Shell change notification
+ * COPYRIGHT:   Copyright 2020 Katayama Hirofumi MZ (katayama.hirofumi.mz(a)gmail.com)
+ */
+#include "shelldesktop.h"
+#include "CDirectoryList.h"
+#include <assert.h>      // for assert
+
+WINE_DEFAULT_DEBUG_CHANNEL(shcn);
+
+BOOL CDirectoryList::ContainsPath(LPCWSTR pszPath) const
+{
+    assert(!PathIsRelativeW(pszPath));
+
+    for (INT i = 0; i < m_items.GetSize(); ++i)
+    {
+        if (m_items[i].IsEmpty())
+            continue;
+
+        if (m_items[i].EqualPath(pszPath))
+            return TRUE; // matched
+    }
+    return FALSE;
+}
+
+BOOL CDirectoryList::AddPath(LPCWSTR pszPath)
+{
+    assert(!PathIsRelativeW(pszPath));
+    return m_items.Add(pszPath);
+}
+
+BOOL CDirectoryList::RenamePath(LPCWSTR pszPath1, LPCWSTR pszPath2)
+{
+    assert(!PathIsRelativeW(pszPath1));
+    assert(!PathIsRelativeW(pszPath2));
+
+    for (INT i = 0; i < m_items.GetSize(); ++i)
+    {
+        if (m_items[i].EqualPath(pszPath1))
+        {
+            // matched
+            m_items[i].SetPath(pszPath2);
+            return TRUE;
+        }
+    }
+    return FALSE;
+}
+
+BOOL CDirectoryList::DeletePath(LPCWSTR pszPath)
+{
+    assert(!PathIsRelativeW(pszPath));
+
+    for (INT i = 0; i < m_items.GetSize(); ++i)
+    {
+        if (m_items[i].EqualPath(pszPath))
+        {
+            // matched
+            m_items[i].SetPath(NULL);
+            return TRUE;
+        }
+    }
+    return FALSE;
+}
+
+BOOL CDirectoryList::AddPathsFromDirectory(LPCWSTR pszDirectoryPath)
+{
+    // get the full path
+    WCHAR szPath[MAX_PATH];
+    lstrcpynW(szPath, pszDirectoryPath, _countof(szPath));
+    assert(!PathIsRelativeW(szPath));
+
+    // is it a directory?
+    if (!PathIsDirectoryW(szPath))
+        return FALSE;
+
+    // add the path
+    if (!AddPath(szPath))
+        return FALSE;
+
+    // enumerate the file items to remember
+    PathAppendW(szPath, L"*");
+    WIN32_FIND_DATAW find;
+    HANDLE hFind = FindFirstFileW(szPath, &find);
+    if (hFind == INVALID_HANDLE_VALUE)
+    {
+        ERR("FindFirstFileW failed\n");
+        return FALSE;
+    }
+
+    LPWSTR pch;
+    do
+    {
+        // ignore "." and ".."
+        pch = find.cFileName;
+        if (pch[0] == L'.' && (pch[1] == 0 || (pch[1] == L'.'
&& pch[2] == 0)))
+            continue;
+
+        // build a path
+        PathRemoveFileSpecW(szPath);
+        if (lstrlenW(szPath) + lstrlenW(find.cFileName) + 1 > MAX_PATH)
+        {
+            ERR("szPath is too long\n");
+            continue;
+        }
+        PathAppendW(szPath, find.cFileName);
+
+        // add the path and do recurse
+        if (find.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
+        {
+            if (m_fRecursive)
+                AddPathsFromDirectory(szPath);
+            else
+                AddPath(szPath);
+        }
+    } while (FindNextFileW(hFind, &find));
+
+    FindClose(hFind);
+
+    return TRUE;
+}
diff --git a/dll/win32/shell32/shelldesktop/CDirectoryList.h
b/dll/win32/shell32/shelldesktop/CDirectoryList.h
new file mode 100644
index 00000000000..1e9559c4b24
--- /dev/null
+++ b/dll/win32/shell32/shelldesktop/CDirectoryList.h
@@ -0,0 +1,93 @@
+#pragma once
+
+#include <atlsimpcoll.h> // for CSimpleArray
+
+//////////////////////////////////////////////////////////////////////////////
+
+// A pathname with info
+class CDirectoryItem
+{
+public:
+    CDirectoryItem() : m_pszPath(NULL)
+    {
+    }
+
+    CDirectoryItem(LPCWSTR pszPath)
+    {
+        m_pszPath = _wcsdup(pszPath);
+    }
+
+    CDirectoryItem(const CDirectoryItem& item)
+        : m_pszPath(_wcsdup(item.m_pszPath))
+    {
+    }
+
+    CDirectoryItem& operator=(const CDirectoryItem& item)
+    {
+        if (this != &item)
+        {
+            free(m_pszPath);
+            m_pszPath = _wcsdup(item.m_pszPath);
+        }
+        return *this;
+    }
+
+    ~CDirectoryItem()
+    {
+        free(m_pszPath);
+    }
+
+    BOOL IsEmpty() const
+    {
+        return m_pszPath == NULL;
+    }
+
+    LPCWSTR GetPath() const
+    {
+        return m_pszPath;
+    }
+
+    void SetPath(LPCWSTR pszPath)
+    {
+        free(m_pszPath);
+        m_pszPath = _wcsdup(pszPath);
+    }
+
+    BOOL EqualPath(LPCWSTR pszPath) const
+    {
+        return m_pszPath != NULL && lstrcmpiW(m_pszPath, pszPath) == 0;
+    }
+
+protected:
+    LPWSTR m_pszPath;    // A full path, malloc'ed
+};
+
+// the directory list
+class CDirectoryList
+{
+public:
+    CDirectoryList() : m_fRecursive(FALSE)
+    {
+    }
+
+    CDirectoryList(LPCWSTR pszDirectoryPath, BOOL fRecursive)
+        : m_fRecursive(fRecursive)
+    {
+        AddPathsFromDirectory(pszDirectoryPath);
+    }
+
+    BOOL ContainsPath(LPCWSTR pszPath) const;
+    BOOL AddPath(LPCWSTR pszPath);
+    BOOL AddPathsFromDirectory(LPCWSTR pszDirectoryPath);
+    BOOL RenamePath(LPCWSTR pszPath1, LPCWSTR pszPath2);
+    BOOL DeletePath(LPCWSTR pszPath);
+
+    void RemoveAll()
+    {
+        m_items.RemoveAll();
+    }
+
+protected:
+    BOOL m_fRecursive;
+    CSimpleArray<CDirectoryItem> m_items;
+};
diff --git a/dll/win32/shell32/shelldesktop/CDirectoryWatcher.cpp
b/dll/win32/shell32/shelldesktop/CDirectoryWatcher.cpp
index d2ed1583558..9a206ac673a 100644
--- a/dll/win32/shell32/shelldesktop/CDirectoryWatcher.cpp
+++ b/dll/win32/shell32/shelldesktop/CDirectoryWatcher.cpp
@@ -71,7 +71,7 @@ static void NTAPI _RequestAllTerminationAPC(ULONG_PTR Parameter)
 CDirectoryWatcher::CDirectoryWatcher(LPCWSTR pszDirectoryPath, BOOL fSubTree)
     : m_fDead(FALSE)
     , m_fRecursive(fSubTree)
-    , m_file_list(pszDirectoryPath, fSubTree)
+    , m_dir_list(pszDirectoryPath, fSubTree)
 {
     TRACE("CDirectoryWatcher::CDirectoryWatcher: %p, '%S'\n", this,
pszDirectoryPath);
 
@@ -139,24 +139,6 @@ void CDirectoryWatcher::ProcessNotification()
     DWORD dwEvent, cbName;
     BOOL fDir;
 
-    // if the watch is recursive
-    if (m_fRecursive)
-    {
-        // get the first change
-        if (!m_file_list.GetFirstChange(szPath))
-            return;
-
-        // then, notify a SHCNE_UPDATEDIR
-        if (lstrcmpiW(m_szDirectoryPath, szPath) != 0)
-            PathRemoveFileSpecW(szPath);
-        NotifyFileSystemChange(SHCNE_UPDATEDIR, szPath, NULL);
-
-        // refresh directory list
-        m_file_list.RemoveAll();
-        m_file_list.AddPathsFromDirectory(m_szDirectoryPath, TRUE);
-        return;
-    }
-
     // for each entry in s_buffer
     szPath[0] = szTempPath[0] = 0;
     for (;;)
@@ -188,37 +170,37 @@ void CDirectoryWatcher::ProcessNotification()
         dwEvent = ConvertActionToEvent(pInfo->Action, fDir);
 
         // convert SHCNE_DELETE to SHCNE_RMDIR if the path is a directory
-        if (!fDir && (dwEvent == SHCNE_DELETE) &&
m_file_list.ContainsPath(szPath, TRUE))
+        if (!fDir && (dwEvent == SHCNE_DELETE) &&
m_dir_list.ContainsPath(szPath))
         {
             fDir = TRUE;
             dwEvent = SHCNE_RMDIR;
         }
 
-        // update m_file_list
+        // update m_dir_list
         switch (dwEvent)
         {
             case SHCNE_MKDIR:
-                if (!m_file_list.AddPath(szPath, 0, TRUE))
+                if (!PathIsDirectoryW(szPath) || !m_dir_list.AddPath(szPath))
                     dwEvent = 0;
                 break;
             case SHCNE_CREATE:
-                if (!m_file_list.AddPath(szPath, INVALID_FILE_SIZE, FALSE))
+                if (!PathFileExistsW(szPath) || PathIsDirectoryW(szPath))
                     dwEvent = 0;
                 break;
             case SHCNE_RENAMEFOLDER:
-                if (!m_file_list.RenamePath(szTempPath, szPath, TRUE))
+                if (!PathIsDirectoryW(szPath) || !m_dir_list.RenamePath(szTempPath,
szPath))
                     dwEvent = 0;
                 break;
             case SHCNE_RENAMEITEM:
-                if (!m_file_list.RenamePath(szTempPath, szPath, FALSE))
+                if (!PathFileExistsW(szPath) || PathIsDirectoryW(szPath))
                     dwEvent = 0;
                 break;
             case SHCNE_RMDIR:
-                if (!m_file_list.DeletePath(szPath, TRUE))
+                if (PathIsDirectoryW(szPath) || !m_dir_list.DeletePath(szPath))
                     dwEvent = 0;
                 break;
             case SHCNE_DELETE:
-                if (!m_file_list.DeletePath(szPath, FALSE))
+                if (PathFileExistsW(szPath))
                     dwEvent = 0;
                 break;
         }
diff --git a/dll/win32/shell32/shelldesktop/CDirectoryWatcher.h
b/dll/win32/shell32/shelldesktop/CDirectoryWatcher.h
index 667d5fac148..a8789b2fdf6 100644
--- a/dll/win32/shell32/shelldesktop/CDirectoryWatcher.h
+++ b/dll/win32/shell32/shelldesktop/CDirectoryWatcher.h
@@ -6,7 +6,7 @@
  */
 #pragma once
 
-#include "CFilePathList.h"
+#include "CDirectoryList.h"
 
 // NOTE: Regard to asynchronous procedure call (APC), please see:
 // https://docs.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-sle…
@@ -31,7 +31,7 @@ public:
 protected:
     BOOL m_fDead;
     BOOL m_fRecursive;
-    CFilePathList m_file_list;
+    CDirectoryList m_dir_list;
     OVERLAPPED m_overlapped;
 
     BOOL CreateAPCThread();
diff --git a/dll/win32/shell32/shelldesktop/CFilePathList.cpp
b/dll/win32/shell32/shelldesktop/CFilePathList.cpp
deleted file mode 100644
index cb4d505a616..00000000000
--- a/dll/win32/shell32/shelldesktop/CFilePathList.cpp
+++ /dev/null
@@ -1,192 +0,0 @@
-/*
- * PROJECT:     shell32
- * LICENSE:     LGPL-2.1-or-later (https://spdx.org/licenses/LGPL-2.1-or-later)
- * PURPOSE:     Shell change notification
- * COPYRIGHT:   Copyright 2020 Katayama Hirofumi MZ (katayama.hirofumi.mz(a)gmail.com)
- */
-#include "shelldesktop.h"
-#include "CFilePathList.h"
-#include <assert.h>      // for assert
-
-WINE_DEFAULT_DEBUG_CHANNEL(shcn);
-
-BOOL CFilePathList::ContainsPath(LPCWSTR pszPath, BOOL fIsDirectory) const
-{
-    assert(!PathIsRelativeW(pszPath));
-
-    for (INT i = 0; i < m_items.GetSize(); ++i)
-    {
-        if (m_items[i].IsEmpty() || fIsDirectory != m_items[i].IsDirectory())
-            continue;
-
-        if (m_items[i].EqualPath(pszPath))
-            return TRUE; // matched
-    }
-    return FALSE;
-}
-
-static DWORD GetSizeOfFile(LPCWSTR pszPath)
-{
-    WIN32_FIND_DATAW find;
-    HANDLE hFind = FindFirstFileW(pszPath, &find);
-    if (hFind == INVALID_HANDLE_VALUE)
-        return FALSE;
-    FindClose(hFind);
-    return find.nFileSizeLow;
-}
-
-BOOL CFilePathList::AddPath(LPCWSTR pszPath, DWORD dwFileSize, BOOL fIsDirectory)
-{
-    assert(!PathIsRelativeW(pszPath));
-
-    if (dwFileSize == INVALID_FILE_SIZE)
-    {
-        dwFileSize = GetSizeOfFile(pszPath);
-    }
-
-    CFilePathItem item(pszPath, dwFileSize, fIsDirectory);
-    return m_items.Add(item);
-}
-
-BOOL CFilePathList::RenamePath(LPCWSTR pszPath1, LPCWSTR pszPath2, BOOL fIsDirectory)
-{
-    assert(!PathIsRelativeW(pszPath1));
-    assert(!PathIsRelativeW(pszPath2));
-
-    for (INT i = 0; i < m_items.GetSize(); ++i)
-    {
-        if (m_items[i].IsDirectory() == fIsDirectory &&
m_items[i].EqualPath(pszPath1))
-        {
-            // matched
-            m_items[i].SetPath(pszPath2);
-            return TRUE;
-        }
-    }
-    return FALSE;
-}
-
-BOOL CFilePathList::DeletePath(LPCWSTR pszPath, BOOL fIsDirectory)
-{
-    assert(!PathIsRelativeW(pszPath));
-
-    for (INT i = 0; i < m_items.GetSize(); ++i)
-    {
-        if (m_items[i].IsDirectory() == fIsDirectory &&
m_items[i].EqualPath(pszPath))
-        {
-            // matched
-            m_items[i].SetPath(NULL);
-            return TRUE;
-        }
-    }
-    return FALSE;
-}
-
-BOOL CFilePathList::AddPathsFromDirectory(LPCWSTR pszDirectoryPath, BOOL fRecursive)
-{
-    // get the full path
-    WCHAR szPath[MAX_PATH];
-    lstrcpynW(szPath, pszDirectoryPath, _countof(szPath));
-    assert(!PathIsRelativeW(szPath));
-
-    // is it a directory?
-    if (!PathIsDirectoryW(szPath))
-        return FALSE;
-
-    // add the path
-    if (!AddPath(szPath, 0, TRUE))
-        return FALSE;
-
-    // enumerate the file items to remember
-    PathAppendW(szPath, L"*");
-    WIN32_FIND_DATAW find;
-    HANDLE hFind = FindFirstFileW(szPath, &find);
-    if (hFind == INVALID_HANDLE_VALUE)
-    {
-        ERR("FindFirstFileW failed\n");
-        return FALSE;
-    }
-
-    do
-    {
-        // ignore "." and ".."
-        if (lstrcmpW(find.cFileName, L".") == 0 ||
-            lstrcmpW(find.cFileName, L"..") == 0)
-        {
-            continue;
-        }
-
-        // build a path
-        PathRemoveFileSpecW(szPath);
-        if (lstrlenW(szPath) + lstrlenW(find.cFileName) + 1 > MAX_PATH)
-        {
-            ERR("szPath is too long\n");
-            continue;
-        }
-        PathAppendW(szPath, find.cFileName);
-
-        BOOL fIsDirectory = !!(find.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY);
-
-        // add the path and do recurse
-        if (fRecursive && fIsDirectory)
-            AddPathsFromDirectory(szPath, fRecursive);
-        else
-            AddPath(szPath, find.nFileSizeLow, fIsDirectory);
-    } while (FindNextFileW(hFind, &find));
-
-    FindClose(hFind);
-
-    return TRUE;
-}
-
-BOOL CFilePathList::GetFirstChange(LPWSTR pszPath) const
-{
-    // validate paths
-    for (INT i = 0; i < m_items.GetSize(); ++i)
-    {
-        if (m_items[i].IsEmpty())
-            continue;
-
-        if (m_items[i].IsDirectory()) // item is a directory
-        {
-            if (!PathIsDirectoryW(m_items[i].GetPath()))
-            {
-                // mismatched
-                lstrcpynW(pszPath, m_items[i].GetPath(), MAX_PATH);
-                return TRUE;
-            }
-        }
-        else // item is a normal file
-        {
-            if (!PathFileExistsW(m_items[i].GetPath()) ||
-                PathIsDirectoryW(m_items[i].GetPath()))
-            {
-                // mismatched
-                lstrcpynW(pszPath, m_items[i].GetPath(), MAX_PATH);
-                return TRUE;
-            }
-        }
-    }
-
-    // check sizes
-    HANDLE hFind;
-    WIN32_FIND_DATAW find;
-    for (INT i = 0; i < m_items.GetSize(); ++i)
-    {
-        if (m_items[i].IsEmpty() || m_items[i].IsDirectory())
-            continue;
-
-        // get size
-        hFind = FindFirstFileW(m_items[i].GetPath(), &find);
-        FindClose(hFind);
-
-        if (hFind == INVALID_HANDLE_VALUE ||
-            find.nFileSizeLow != m_items[i].GetSize())
-        {
-            // different size
-            lstrcpynW(pszPath, m_items[i].GetPath(), MAX_PATH);
-            return TRUE;
-        }
-    }
-
-    return FALSE;
-}
diff --git a/dll/win32/shell32/shelldesktop/CFilePathList.h
b/dll/win32/shell32/shelldesktop/CFilePathList.h
deleted file mode 100644
index 815fdaba4a3..00000000000
--- a/dll/win32/shell32/shelldesktop/CFilePathList.h
+++ /dev/null
@@ -1,108 +0,0 @@
-#pragma once
-
-#include <atlsimpcoll.h> // for CSimpleArray
-
-//////////////////////////////////////////////////////////////////////////////
-
-// A pathname with info
-class CFilePathItem
-{
-public:
-    CFilePathItem() : m_pszPath(NULL)
-    {
-    }
-
-    CFilePathItem(LPCWSTR pszPath, DWORD dwFileSize, BOOL IsDirectory)
-    {
-        m_pszPath = _wcsdup(pszPath);
-        m_dwFileSize = dwFileSize;
-        m_fIsDirectory = IsDirectory;
-    }
-
-    CFilePathItem(const CFilePathItem& item)
-        : m_pszPath(_wcsdup(item.m_pszPath))
-        , m_dwFileSize(item.m_dwFileSize)
-        , m_fIsDirectory(item.m_fIsDirectory)
-    {
-    }
-
-    CFilePathItem& operator=(const CFilePathItem& item)
-    {
-        free(m_pszPath);
-        m_pszPath = _wcsdup(item.m_pszPath);
-        m_dwFileSize = item.m_dwFileSize;
-        m_fIsDirectory = item.m_fIsDirectory;
-        return *this;
-    }
-
-    ~CFilePathItem()
-    {
-        free(m_pszPath);
-    }
-
-    BOOL IsEmpty() const
-    {
-        return m_pszPath == NULL;
-    }
-
-    BOOL IsDirectory() const
-    {
-        return m_fIsDirectory;
-    }
-
-    LPCWSTR GetPath() const
-    {
-        return m_pszPath;
-    }
-
-    void SetPath(LPCWSTR pszPath)
-    {
-        free(m_pszPath);
-        m_pszPath = _wcsdup(pszPath);
-    }
-
-    BOOL EqualPath(LPCWSTR pszPath) const
-    {
-        return m_pszPath != NULL && lstrcmpiW(m_pszPath, pszPath) == 0;
-    }
-
-    DWORD GetSize() const
-    {
-        return m_dwFileSize;
-    }
-
-protected:
-    LPWSTR m_pszPath;    // A full path, malloc'ed
-    DWORD m_dwFileSize;  // the size of a file
-    BOOL m_fIsDirectory; // is it a directory?
-};
-
-// the file list
-class CFilePathList
-{
-public:
-    CFilePathList()
-    {
-    }
-
-    CFilePathList(LPCWSTR pszDirectoryPath, BOOL fRecursive)
-    {
-        AddPathsFromDirectory(pszDirectoryPath, fRecursive);
-    }
-
-    BOOL ContainsPath(LPCWSTR pszPath, BOOL fIsDirectory) const;
-    BOOL GetFirstChange(LPWSTR pszPath) const;
-
-    BOOL AddPath(LPCWSTR pszPath, DWORD dwFileSize, BOOL fIsDirectory);
-    BOOL AddPathsFromDirectory(LPCWSTR pszDirectoryPath, BOOL fRecursive);
-    BOOL RenamePath(LPCWSTR pszPath1, LPCWSTR pszPath2, BOOL fIsDirectory);
-    BOOL DeletePath(LPCWSTR pszPath, BOOL fIsDirectory);
-
-    void RemoveAll()
-    {
-        m_items.RemoveAll();
-    }
-
-protected:
-    CSimpleArray<CFilePathItem> m_items;
-};
diff --git a/dll/win32/shell32/shelldesktop/CMakeLists.txt
b/dll/win32/shell32/shelldesktop/CMakeLists.txt
index 498eb70e024..2ea167b95f7 100644
--- a/dll/win32/shell32/shelldesktop/CMakeLists.txt
+++ b/dll/win32/shell32/shelldesktop/CMakeLists.txt
@@ -13,7 +13,7 @@ list(APPEND SOURCE
     CChangeNotifyServer.cpp
     CDesktopBrowser.cpp
     CDirectoryWatcher.cpp
-    CFilePathList.cpp
+    CDirectoryList.cpp
     dde.cpp)
 
 add_library(shelldesktop ${SOURCE})