diff --git a/laf b/laf index 7015a5b8d..c4ffd09f7 160000 --- a/laf +++ b/laf @@ -1 +1 @@ -Subproject commit 7015a5b8d50d09470031c9b8cdfedceff614be9b +Subproject commit c4ffd09f718eb4c5f161e98344aab51cd5c1f379 diff --git a/src/app/file_system.cpp b/src/app/file_system.cpp index 70ad2d357..f9bad76a4 100644 --- a/src/app/file_system.cpp +++ b/src/app/file_system.cpp @@ -30,6 +30,8 @@ #include #ifdef _WIN32 + #include "base/win/comptr.h" + #include #include @@ -64,8 +66,8 @@ FileItemMap* fileitems_map = nullptr; unsigned int current_file_system_version = 0; #ifdef _WIN32 - IMalloc* shl_imalloc = nullptr; - IShellFolder* shl_idesktop = nullptr; + base::ComPtr shl_imalloc; + base::ComPtr shl_idesktop; #endif // a position in the file-system @@ -213,17 +215,14 @@ FileSystemModule::~FileSystemModule() fileitems_map->clear(); #ifdef _WIN32 - // relase desktop IShellFolder interface - shl_idesktop->Release(); - - // release IMalloc interface - shl_imalloc->Release(); - shl_imalloc = NULL; + // Release interfaces + shl_idesktop.reset(); + shl_imalloc.reset(); #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 pFolder; HRESULT hr; - if (this == rootitem) + if (this == rootitem) { pFolder = shl_idesktop; + } else { - hr = shl_idesktop->BindToObject(m_fullpidl, - NULL, IID_IShellFolder, (LPVOID *)&pFolder); + hr = shl_idesktop->BindToObject( + m_fullpidl, nullptr, + IID_IShellFolder, (LPVOID *)&pFolder); if (hr != S_OK) - pFolder = NULL; + pFolder = nullptr; } - if (pFolder != NULL) { - IEnumIDList *pEnum = NULL; + if (pFolder) { + base::ComPtr pEnum; ULONG c, fetched; // Get the interface to enumerate subitems @@ -459,7 +460,7 @@ const FileItemList& FileItem::children() reinterpret_cast(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; cGetAttributesOf(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 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 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) diff --git a/src/ui/menu.cpp b/src/ui/menu.cpp index 78eceab8d..722697899 100644 --- a/src/ui/menu.cpp +++ b/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 = new MenuBaseData; - return m_base; + m_base.reset(new MenuBaseData); + return m_base.get(); } Menu* MenuItem::getSubmenu() @@ -484,18 +482,29 @@ bool MenuBox::onProcessMessage(Message* msg) switch (msg->type()) { - case kMouseMoveMessage: - if (!get_base(this)->was_clicked) + case kMouseMoveMessage: { + 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(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(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(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(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->is_processing == false); + ASSERT(base); + if (base) { + ASSERT(base->is_processing == false); - // Start processing - base->is_processing = true; + // Start processing + base->is_processing = true; + } } } @@ -1349,17 +1397,23 @@ void MenuBox::stopFilteringMouseDown() void MenuBox::cancelMenuLoop() { Menu* menu = getMenu(); - if (menu) { - // Do not close the popup menus if we're already processing - // open/close popup messages. - if (get_base(this)->is_processing) - return; + if (!menu) + return; - menu->closeAll(); + MenuBaseData* base = get_base(this); + ASSERT(base); + if (!base) + return; - // Lost focus - Manager::getDefault()->freeFocus(); - } + // Do not close the popup menus if we're already processing + // open/close popup messages. + if (base->is_processing) + return; + + menu->closeAll(); + + // Lost focus + Manager::getDefault()->freeFocus(); } void MenuItem::executeClick() diff --git a/src/ui/menu.h b/src/ui/menu.h index 596c2dc79..4318eae28 100644 --- a/src/ui/menu.h +++ b/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 m_base; friend class Menu; friend class MenuItem;