Merge branch 'main' into beta

laf

@@ -1 +1 @@
-Subproject commit 7015a5b8d50d09470031c9b8cdfedceff614be9b
+Subproject commit c4ffd09f718eb4c5f161e98344aab51cd5c1f379

src/app/file_system.cpp

@@ -30,6 +30,8 @@
 #include <vector>
 
 #ifdef _WIN32
+  #include "base/win/comptr.h"
+
   #include <windows.h>
 
   #include <shlobj.h>
@@ -64,8 +66,8 @@ FileItemMap* fileitems_map = nullptr;
 unsigned int current_file_system_version = 0;
 
 #ifdef _WIN32
-  IMalloc* shl_imalloc = nullptr;
+  base::ComPtr<IMalloc> shl_imalloc;
-  IShellFolder* shl_idesktop = nullptr;
+  base::ComPtr<IShellFolder> shl_idesktop;
 #endif
 
 // a position in the file-system
@@ -213,17 +215,14 @@ FileSystemModule::~FileSystemModule()
   fileitems_map->clear();
 
 #ifdef _WIN32
-  // relase desktop IShellFolder interface
+  // Release interfaces
-  shl_idesktop->Release();
+  shl_idesktop.reset();
-
+  shl_imalloc.reset();
-  // release IMalloc interface
-  shl_imalloc->Release();
-  shl_imalloc = NULL;
 #endif
 
   delete fileitems_map;
 
-  m_instance = NULL;
+  m_instance = nullptr;
 }
 
 FileSystemModule* FileSystemModule::instance()
@@ -437,21 +436,23 @@ const FileItemList& FileItem::children()
     //LOG("FS: Loading files for %p (%s)\n", fileitem, fileitem->displayname);
 #ifdef _WIN32
     {
-      IShellFolder* pFolder = NULL;
+      base::ComPtr<IShellFolder> pFolder;
       HRESULT hr;
 
-      if (this == rootitem)
+      if (this == rootitem) {
         pFolder = shl_idesktop;
+      }
       else {
-        hr = shl_idesktop->BindToObject(m_fullpidl,
+        hr = shl_idesktop->BindToObject(
-          NULL, IID_IShellFolder, (LPVOID *)&pFolder);
+          m_fullpidl, nullptr,
+          IID_IShellFolder, (LPVOID *)&pFolder);
 
         if (hr != S_OK)
-          pFolder = NULL;
+          pFolder = nullptr;
       }
 
-      if (pFolder != NULL) {
+      if (pFolder) {
-        IEnumIDList *pEnum = NULL;
+        base::ComPtr<IEnumIDList> pEnum;
         ULONG c, fetched;
 
         // Get the interface to enumerate subitems
@@ -459,7 +460,7 @@ const FileItemList& FileItem::children()
           reinterpret_cast<HWND>(os::instance()->defaultWindow()->nativeHandle()),
           SHCONTF_FOLDERS | SHCONTF_NONFOLDERS, &pEnum);
 
-        if (hr == S_OK && pEnum != NULL) {
+        if (hr == S_OK && pEnum) {
           LPITEMIDLIST itempidl[256];
           SFGAOF attribs[256];
 
@@ -469,7 +470,7 @@ const FileItemList& FileItem::children()
             // item is file or a folder
             for (c=0; c<fetched; ++c) {
               attribs[c] = SFGAO_FOLDER;
-              pFolder->GetAttributesOf(1, (LPCITEMIDLIST *)itempidl, attribs+c);
+              pFolder->GetAttributesOf(1, (LPCITEMIDLIST*)itempidl, attribs+c);
             }
 
             // Generate the FileItems
@@ -496,12 +497,7 @@ const FileItemList& FileItem::children()
               insertChildSorted(child);
             }
           }
-
-          pEnum->Release();
         }
-
-        if (pFolder != shl_idesktop)
-          pFolder->Release();
       }
     }
 #else
@@ -693,7 +689,7 @@ static SFGAOF get_pidl_attrib(FileItem* fileitem, SFGAOF attrib)
 
   HRESULT hr;
 
-  IShellFolder* pFolder = nullptr;
+  base::ComPtr<IShellFolder> pFolder;
   if (fileitem->m_parent == rootitem)
     pFolder = shl_idesktop;
   else {
@@ -708,8 +704,6 @@ static SFGAOF get_pidl_attrib(FileItem* fileitem, SFGAOF attrib)
     hr = pFolder->GetAttributesOf(1, (LPCITEMIDLIST*)&fileitem->m_pidl, &attrib2);
     if (hr == S_OK)
       attrib = attrib2;
-    if (pFolder && pFolder != shl_idesktop)
-      pFolder->Release();
   }
   return attrib;
 }
@@ -719,7 +713,7 @@ static void update_by_pidl(FileItem* fileitem, SFGAOF attrib)
 {
   STRRET strret;
   WCHAR pszName[MAX_PATH];
-  IShellFolder* pFolder = NULL;
+  base::ComPtr<IShellFolder> pFolder;
   HRESULT hr;
 
   if (fileitem == rootitem)
@@ -729,7 +723,7 @@ static void update_by_pidl(FileItem* fileitem, SFGAOF attrib)
     hr = shl_idesktop->BindToObject(fileitem->m_parent->m_fullpidl,
                                     nullptr, IID_IShellFolder, (LPVOID*)&pFolder);
     if (hr != S_OK)
-      pFolder = NULL;
+      pFolder = nullptr;
   }
 
   // Get the file name
@@ -774,10 +768,6 @@ static void update_by_pidl(FileItem* fileitem, SFGAOF attrib)
   else {
     fileitem->m_displayname = base::get_file_name(fileitem->m_filename);
   }
-
-  if (pFolder && pFolder != shl_idesktop) {
-    pFolder->Release();
-  }
 }
 
 static LPITEMIDLIST concat_pidl(LPITEMIDLIST pidlHead, LPITEMIDLIST pidlTail)

src/ui/menu.cpp

@@ -233,7 +233,6 @@ MenuBox::MenuBox(WidgetType type)
 MenuBox::~MenuBox()
 {
   stopFilteringMouseDown();
-  delete m_base;
 }
 
 //////////////////////////////////////////////////////////////////////
@@ -288,9 +287,8 @@ Menu* MenuBox::getMenu()
 
 MenuBaseData* MenuBox::createBase()
 {
-  delete m_base;
+  m_base.reset(new MenuBaseData);
-  m_base = new MenuBaseData;
+  return m_base.get();
-  return m_base;
 }
 
 Menu* MenuItem::getSubmenu()
@@ -484,18 +482,29 @@ bool MenuBox::onProcessMessage(Message* msg)
 
   switch (msg->type()) {
 
-    case kMouseMoveMessage:
+    case kMouseMoveMessage: {
-      if (!get_base(this)->was_clicked)
+      MenuBaseData* base = get_base(this);
+      ASSERT(base);
+      if (!base)
         break;
 
-      // Fall through
+      if (!base->was_clicked)
+        break;
+
+      //[[fallthrough]];
+    }
 
     case kMouseDownMessage:
     case kDoubleClickMessage:
       if (menu && msg->display()) {
         ASSERT(menu->parent() == this);
 
-        if (get_base(this)->is_processing)
+        MenuBaseData* base = get_base(this);
+        ASSERT(base);
+        if (!base)
+          break;
+
+        if (base->is_processing)
           break;
 
         const gfx::Point mousePos = static_cast<MouseMessage*>(msg)->position();
@@ -556,11 +565,11 @@ bool MenuBox::onProcessMessage(Message* msg)
 
                 // Set this flag to false so the submenu is not open
                 // again on kMouseMoveMessage.
-                get_base(this)->was_clicked = false;
+                base->was_clicked = false;
               }
             }
           }
-          else if (!get_base(this)->was_clicked) {
+          else if (!base->was_clicked) {
             menu->unhighlightItem();
           }
         }
@@ -569,7 +578,12 @@ bool MenuBox::onProcessMessage(Message* msg)
 
     case kMouseLeaveMessage:
       if (menu) {
-        if (get_base(this)->is_processing)
+        MenuBaseData* base = get_base(this);
+        ASSERT(base);
+        if (!base)
+          break;
+
+        if (base->is_processing)
           break;
 
         MenuItem* highlight = menu->getHighlightedItem();
@@ -580,7 +594,12 @@ bool MenuBox::onProcessMessage(Message* msg)
 
     case kMouseUpMessage:
       if (menu) {
-        if (get_base(this)->is_processing)
+        MenuBaseData* base = get_base(this);
+        ASSERT(base);
+        if (!base)
+          break;
+
+        if (base->is_processing)
           break;
 
         // The item is highlighted and not opened (and the timer to open the submenu is stopped)
@@ -598,10 +617,15 @@ bool MenuBox::onProcessMessage(Message* msg)
       if (menu) {
         MenuItem* selected;
 
-        if (get_base(this)->is_processing)
+        MenuBaseData* base = get_base(this);
+        ASSERT(base);
+        if (!base)
           break;
 
-        get_base(this)->was_clicked = false;
+        if (base->is_processing)
+          break;
+
+        base->was_clicked = false;
 
         // Check for ALT+some underlined letter
         if (((this->type() == kMenuBoxWidget) && (msg->modifiers() == kKeyNoneModifier || // <-- Inside menu-boxes we can use letters without Alt modifier pressed
@@ -738,6 +762,9 @@ bool MenuBox::onProcessMessage(Message* msg)
                 else if (menu->m_menuitem) {
                   // Get the root menu
                   MenuBox* root = get_base_menubox(this);
+                  ASSERT(root);
+                  if (!root)
+                    break;
                   menu = root->getMenu();
 
                   // Go to the next item in the root
@@ -859,9 +886,12 @@ bool MenuItem::onProcessMessage(Message* msg)
         validateItem();
 
         MenuBaseData* base = get_base(this);
+        ASSERT(base);
+        if (!base)
+          break;
+
         bool select_first = static_cast<OpenMenuItemMessage*>(msg)->select_first();
 
-        ASSERT(base != nullptr);
         ASSERT(base->is_processing);
         ASSERT(hasSubmenu());
 
@@ -931,8 +961,10 @@ bool MenuItem::onProcessMessage(Message* msg)
       else if (msg->type() == kCloseMenuItemMessage) {
         bool last_of_close_chain = static_cast<CloseMenuItemMessage*>(msg)->last_of_close_chain();
         MenuBaseData* base = get_base(this);
+        ASSERT(base);
+        if (!base)
+          break;
 
-        ASSERT(base != nullptr);
         ASSERT(base->is_processing);
 
         MenuBox* menubox = m_submenu_menubox;
@@ -977,6 +1009,9 @@ bool MenuItem::onProcessMessage(Message* msg)
     case kTimerMessage:
       if (static_cast<TimerMessage*>(msg)->timer() == m_submenu_timer.get()) {
         MenuBaseData* base = get_base(this);
+        ASSERT(base);
+        if (!base)
+          break;
 
         ASSERT(hasSubmenu());
 
@@ -1056,6 +1091,12 @@ static MenuBox* get_base_menubox(Widget* widget)
         ASSERT(menu != nullptr);
         ASSERT(menu->getOwnerMenuItem() != nullptr);
 
+        // We have received a crash report where the "menu" variable
+        // can be nullptr in the kMouseDownMessage message processing
+        // from MenuBox::onProcessMessage().
+        if (menu == nullptr)
+          return nullptr;
+
         widget = menu->getOwnerMenuItem();
       }
     }
@@ -1150,7 +1191,10 @@ void Menu::highlightItem(MenuItem* menuitem, bool click, bool open_submenu, bool
           menuitem->openSubmenu(select_first_child);
 
         // The mouse was clicked
-        get_base(menuitem)->was_clicked = true;
+        MenuBaseData* base = get_base(menuitem);
+        ASSERT(base);
+        if (base)
+          base->was_clicked = true;
       }
     }
     // Execute menuitem action
@@ -1207,7 +1251,9 @@ void MenuItem::openSubmenu(bool select_first)
 
   // Get the 'base'
   MenuBaseData* base = get_base(this);
-  ASSERT(base != nullptr);
+  ASSERT(base);
+  if (!base)
+    return;
   ASSERT(base->is_processing == false);
 
   // Reset flags
@@ -1253,11 +1299,13 @@ void MenuItem::closeSubmenu(bool last_of_close_chain)
   if (last_of_close_chain) {
     // Get the 'base'
     base = get_base(this);
-    ASSERT(base != nullptr);
+    ASSERT(base);
-    ASSERT(base->is_processing == false);
+    if (base) {
+      ASSERT(base->is_processing == false);
 
-    // Start processing
+      // Start processing
-    base->is_processing = true;
+      base->is_processing = true;
+    }
   }
 }
 
@@ -1349,17 +1397,23 @@ void MenuBox::stopFilteringMouseDown()
 void MenuBox::cancelMenuLoop()
 {
   Menu* menu = getMenu();
-  if (menu) {
+  if (!menu)
-    // Do not close the popup menus if we're already processing
+    return;
-    // open/close popup messages.
-    if (get_base(this)->is_processing)
-      return;
 
-    menu->closeAll();
+  MenuBaseData* base = get_base(this);
+  ASSERT(base);
+  if (!base)
+    return;
 
-    // Lost focus
+  // Do not close the popup menus if we're already processing
-    Manager::getDefault()->freeFocus();
+  // open/close popup messages.
-  }
+  if (base->is_processing)
+    return;
+
+  menu->closeAll();
+
+  // Lost focus
+  Manager::getDefault()->freeFocus();
 }
 
 void MenuItem::executeClick()

src/ui/menu.h

@@ -71,7 +71,7 @@ namespace ui {
     void setMenu(Menu* menu);
 
     MenuBaseData* getBase() {
-      return m_base;
+      return m_base.get();
     }
 
     // Closes all menu-boxes and goes back to the normal state of the
@@ -89,7 +89,7 @@ namespace ui {
     void startFilteringMouseDown();
     void stopFilteringMouseDown();
 
-    MenuBaseData* m_base;
+    std::unique_ptr<MenuBaseData> m_base;
 
     friend class Menu;
     friend class MenuItem;