From d3448fa204a648d61d07f12ecc982841160380d2 Mon Sep 17 00:00:00 2001 From: Andrey Kleshchev Date: Tue, 30 Jan 2024 01:50:15 +0200 Subject: BugSplat Crash #1409959 onTopLost onTopLost crashed 1. It contradicts callstack, but clearPopups() definetely has an issue due to not checking the pointer prior to calling onTopLost 2. According to callstack, crash happened around ~LLFolderViewFolder and while it does call removePopup for itself, it isn't a popup, the only one in the list would be the renamer, which calls back to parent, so made sure to secure it. 3. mFlashTimer was never deleted 4. Some explicit cleanup for TopLost --- indra/newview/llpopupview.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'indra/newview/llpopupview.cpp') diff --git a/indra/newview/llpopupview.cpp b/indra/newview/llpopupview.cpp index d1a9ca229f..49645eec11 100644 --- a/indra/newview/llpopupview.cpp +++ b/indra/newview/llpopupview.cpp @@ -260,12 +260,12 @@ void LLPopupView::clearPopups() popup_it != mPopups.end();) { LLView* popup = popup_it->get(); + if (popup) + { + popup->onTopLost(); + } - popup_list_t::iterator cur_popup_it = popup_it; - ++popup_it; - - mPopups.erase(cur_popup_it); - popup->onTopLost(); + popup_it = mPopups.erase(popup_it); } } -- cgit v1.2.3 From d47c6536820d1ed6e373147678dd0fab90e80ab8 Mon Sep 17 00:00:00 2001 From: Andrey Kleshchev Date: Thu, 29 Feb 2024 23:00:08 +0200 Subject: triage#105 clearPopups() crash onTopLost can result in popup being removed or potentially removing more than one popup. --- indra/newview/llpopupview.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'indra/newview/llpopupview.cpp') diff --git a/indra/newview/llpopupview.cpp b/indra/newview/llpopupview.cpp index 49645eec11..aadef9a308 100644 --- a/indra/newview/llpopupview.cpp +++ b/indra/newview/llpopupview.cpp @@ -256,16 +256,16 @@ void LLPopupView::removePopup(LLView* popup) void LLPopupView::clearPopups() { - for (popup_list_t::iterator popup_it = mPopups.begin(); - popup_it != mPopups.end();) + while (!mPopups.empty()) { - LLView* popup = popup_it->get(); + popup_list_t::iterator popup_it = mPopups.begin(); + LLView* popup = popup_it->get(); + // Remove before notifying in case it will cause removePopup + mPopups.erase(popup_it); if (popup) { popup->onTopLost(); } - - popup_it = mPopups.erase(popup_it); } } -- cgit v1.2.3 From 1b68f71348ecf3983b76b40d7940da8377f049b7 Mon Sep 17 00:00:00 2001 From: Andrey Lihatskiy Date: Mon, 29 Apr 2024 07:43:28 +0300 Subject: #824 Process source files in bulk: replace tabs with spaces, convert CRLF to LF, and trim trailing whitespaces as needed --- indra/newview/llpopupview.cpp | 330 +++++++++++++++++++++--------------------- 1 file changed, 165 insertions(+), 165 deletions(-) (limited to 'indra/newview/llpopupview.cpp') diff --git a/indra/newview/llpopupview.cpp b/indra/newview/llpopupview.cpp index d1a9ca229f..353d261ad1 100644 --- a/indra/newview/llpopupview.cpp +++ b/indra/newview/llpopupview.cpp @@ -1,25 +1,25 @@ -/** +/** * @file llpopupview.cpp * @brief Holds transient popups * * $LicenseInfo:firstyear=2001&license=viewerlgpl$ * Second Life Viewer Source Code * Copyright (C) 2010, Linden Research, Inc. - * + * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public * License as published by the Free Software Foundation; * version 2.1 of the License only. - * + * * This library is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * Lesser General Public License for more details. - * + * * You should have received a copy of the GNU Lesser General Public * License along with this library; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA - * + * * Linden Research, Inc., 945 Battery Street, San Francisco, CA 94111 USA * $/LicenseInfo$ */ @@ -31,241 +31,241 @@ static LLPanelInjector r("popup_holder"); bool view_visible_and_enabled(LLView* viewp) { - return viewp->getVisible() && viewp->getEnabled(); + return viewp->getVisible() && viewp->getEnabled(); } bool view_visible(LLView* viewp) { - return viewp->getVisible(); + return viewp->getVisible(); } LLPopupView::LLPopupView(const LLPopupView::Params& p) : LLPanel(p) { - // register ourself as handler of UI popups - LLUI::getInstance()->setPopupFuncs(boost::bind(&LLPopupView::addPopup, this, _1), boost::bind(&LLPopupView::removePopup, this, _1), boost::bind(&LLPopupView::clearPopups, this)); + // register ourself as handler of UI popups + LLUI::getInstance()->setPopupFuncs(boost::bind(&LLPopupView::addPopup, this, _1), boost::bind(&LLPopupView::removePopup, this, _1), boost::bind(&LLPopupView::clearPopups, this)); } LLPopupView::~LLPopupView() { - // set empty callback function so we can't handle popups anymore - LLUI::getInstance()->setPopupFuncs(LLUI::add_popup_t(), LLUI::remove_popup_t(), LLUI::clear_popups_t()); + // set empty callback function so we can't handle popups anymore + LLUI::getInstance()->setPopupFuncs(LLUI::add_popup_t(), LLUI::remove_popup_t(), LLUI::clear_popups_t()); } void LLPopupView::draw() { - S32 screen_x, screen_y; - - // remove dead popups - for (popup_list_t::iterator popup_it = mPopups.begin(); - popup_it != mPopups.end();) - { - if (!popup_it->get()) - { - mPopups.erase(popup_it++); - } - else - { - popup_it++; - } - } - - // draw in reverse order (most recent is on top) - for (popup_list_t::reverse_iterator popup_it = mPopups.rbegin(); - popup_it != mPopups.rend();) - { - LLView* popup = popup_it->get(); - - if (popup->getVisible()) - { - popup->localPointToScreen(0, 0, &screen_x, &screen_y); - - LLUI::pushMatrix(); - { - LLUI::translate( (F32) screen_x, (F32) screen_y); - popup->draw(); - } - LLUI::popMatrix(); - } - ++popup_it; - } - - LLPanel::draw(); + S32 screen_x, screen_y; + + // remove dead popups + for (popup_list_t::iterator popup_it = mPopups.begin(); + popup_it != mPopups.end();) + { + if (!popup_it->get()) + { + mPopups.erase(popup_it++); + } + else + { + popup_it++; + } + } + + // draw in reverse order (most recent is on top) + for (popup_list_t::reverse_iterator popup_it = mPopups.rbegin(); + popup_it != mPopups.rend();) + { + LLView* popup = popup_it->get(); + + if (popup->getVisible()) + { + popup->localPointToScreen(0, 0, &screen_x, &screen_y); + + LLUI::pushMatrix(); + { + LLUI::translate( (F32) screen_x, (F32) screen_y); + popup->draw(); + } + LLUI::popMatrix(); + } + ++popup_it; + } + + LLPanel::draw(); } -BOOL LLPopupView::handleMouseEvent(boost::function func, - boost::function predicate, - S32 x, S32 y, - bool close_popups) +BOOL LLPopupView::handleMouseEvent(boost::function func, + boost::function predicate, + S32 x, S32 y, + bool close_popups) { - BOOL handled = FALSE; - - // make a copy of list of popups, in case list is modified during mouse event handling - popup_list_t popups(mPopups); - for (popup_list_t::iterator popup_it = popups.begin(), popup_end = popups.end(); - popup_it != popup_end; - ++popup_it) - { - LLView* popup = popup_it->get(); - if (!popup - || !predicate(popup)) - { - continue; - } - - S32 popup_x, popup_y; - if (localPointToOtherView(x, y, &popup_x, &popup_y, popup) - && popup->pointInView(popup_x, popup_y)) - { - if (func(popup, popup_x, popup_y)) - { - handled = TRUE; - break; - } - } - - if (close_popups) - { - mPopups.remove(*popup_it); - popup->onTopLost(); - } - } - - return handled; + BOOL handled = FALSE; + + // make a copy of list of popups, in case list is modified during mouse event handling + popup_list_t popups(mPopups); + for (popup_list_t::iterator popup_it = popups.begin(), popup_end = popups.end(); + popup_it != popup_end; + ++popup_it) + { + LLView* popup = popup_it->get(); + if (!popup + || !predicate(popup)) + { + continue; + } + + S32 popup_x, popup_y; + if (localPointToOtherView(x, y, &popup_x, &popup_y, popup) + && popup->pointInView(popup_x, popup_y)) + { + if (func(popup, popup_x, popup_y)) + { + handled = TRUE; + break; + } + } + + if (close_popups) + { + mPopups.remove(*popup_it); + popup->onTopLost(); + } + } + + return handled; } BOOL LLPopupView::handleMouseDown(S32 x, S32 y, MASK mask) { - BOOL handled = handleMouseEvent(boost::bind(&LLMouseHandler::handleMouseDown, _1, _2, _3, mask), view_visible_and_enabled, x, y, true); - if (!handled) - { - handled = LLPanel::handleMouseDown(x, y, mask); - } - return handled; + BOOL handled = handleMouseEvent(boost::bind(&LLMouseHandler::handleMouseDown, _1, _2, _3, mask), view_visible_and_enabled, x, y, true); + if (!handled) + { + handled = LLPanel::handleMouseDown(x, y, mask); + } + return handled; } BOOL LLPopupView::handleMouseUp(S32 x, S32 y, MASK mask) { - BOOL handled = handleMouseEvent(boost::bind(&LLMouseHandler::handleMouseUp, _1, _2, _3, mask), view_visible_and_enabled, x, y, false); - if (!handled) - { - handled = LLPanel::handleMouseUp(x, y, mask); - } - return handled; + BOOL handled = handleMouseEvent(boost::bind(&LLMouseHandler::handleMouseUp, _1, _2, _3, mask), view_visible_and_enabled, x, y, false); + if (!handled) + { + handled = LLPanel::handleMouseUp(x, y, mask); + } + return handled; } BOOL LLPopupView::handleMiddleMouseDown(S32 x, S32 y, MASK mask) { - BOOL handled = handleMouseEvent(boost::bind(&LLMouseHandler::handleMiddleMouseDown, _1, _2, _3, mask), view_visible_and_enabled, x, y, true); - if (!handled) - { - handled = LLPanel::handleMiddleMouseDown(x, y, mask); - } - return handled; + BOOL handled = handleMouseEvent(boost::bind(&LLMouseHandler::handleMiddleMouseDown, _1, _2, _3, mask), view_visible_and_enabled, x, y, true); + if (!handled) + { + handled = LLPanel::handleMiddleMouseDown(x, y, mask); + } + return handled; } BOOL LLPopupView::handleMiddleMouseUp(S32 x, S32 y, MASK mask) { - BOOL handled = handleMouseEvent(boost::bind(&LLMouseHandler::handleMiddleMouseUp, _1, _2, _3, mask), view_visible_and_enabled, x, y, false); - if (!handled) - { - handled = LLPanel::handleMiddleMouseUp(x, y, mask); - } - return handled; + BOOL handled = handleMouseEvent(boost::bind(&LLMouseHandler::handleMiddleMouseUp, _1, _2, _3, mask), view_visible_and_enabled, x, y, false); + if (!handled) + { + handled = LLPanel::handleMiddleMouseUp(x, y, mask); + } + return handled; } BOOL LLPopupView::handleRightMouseDown(S32 x, S32 y, MASK mask) { - BOOL handled = handleMouseEvent(boost::bind(&LLMouseHandler::handleRightMouseDown, _1, _2, _3, mask), view_visible_and_enabled, x, y, true); - if (!handled) - { - handled = LLPanel::handleRightMouseDown(x, y, mask); - } - return handled; + BOOL handled = handleMouseEvent(boost::bind(&LLMouseHandler::handleRightMouseDown, _1, _2, _3, mask), view_visible_and_enabled, x, y, true); + if (!handled) + { + handled = LLPanel::handleRightMouseDown(x, y, mask); + } + return handled; } BOOL LLPopupView::handleRightMouseUp(S32 x, S32 y, MASK mask) { - BOOL handled = handleMouseEvent(boost::bind(&LLMouseHandler::handleRightMouseUp, _1, _2, _3, mask), view_visible_and_enabled, x, y, false); - if (!handled) - { - handled = LLPanel::handleRightMouseUp(x, y, mask); - } - return handled; + BOOL handled = handleMouseEvent(boost::bind(&LLMouseHandler::handleRightMouseUp, _1, _2, _3, mask), view_visible_and_enabled, x, y, false); + if (!handled) + { + handled = LLPanel::handleRightMouseUp(x, y, mask); + } + return handled; } BOOL LLPopupView::handleDoubleClick(S32 x, S32 y, MASK mask) { - BOOL handled = handleMouseEvent(boost::bind(&LLMouseHandler::handleDoubleClick, _1, _2, _3, mask), view_visible_and_enabled, x, y, false); - if (!handled) - { - handled = LLPanel::handleDoubleClick(x, y, mask); - } - return handled; + BOOL handled = handleMouseEvent(boost::bind(&LLMouseHandler::handleDoubleClick, _1, _2, _3, mask), view_visible_and_enabled, x, y, false); + if (!handled) + { + handled = LLPanel::handleDoubleClick(x, y, mask); + } + return handled; } BOOL LLPopupView::handleHover(S32 x, S32 y, MASK mask) { - BOOL handled = handleMouseEvent(boost::bind(&LLMouseHandler::handleHover, _1, _2, _3, mask), view_visible_and_enabled, x, y, false); - if (!handled) - { - handled = LLPanel::handleHover(x, y, mask); - } - return handled; + BOOL handled = handleMouseEvent(boost::bind(&LLMouseHandler::handleHover, _1, _2, _3, mask), view_visible_and_enabled, x, y, false); + if (!handled) + { + handled = LLPanel::handleHover(x, y, mask); + } + return handled; } BOOL LLPopupView::handleScrollWheel(S32 x, S32 y, S32 clicks) { - BOOL handled = handleMouseEvent(boost::bind(&LLMouseHandler::handleScrollWheel, _1, _2, _3, clicks), view_visible_and_enabled, x, y, false); - if (!handled) - { - handled = LLPanel::handleScrollWheel(x, y, clicks); - } - return handled; + BOOL handled = handleMouseEvent(boost::bind(&LLMouseHandler::handleScrollWheel, _1, _2, _3, clicks), view_visible_and_enabled, x, y, false); + if (!handled) + { + handled = LLPanel::handleScrollWheel(x, y, clicks); + } + return handled; } BOOL LLPopupView::handleToolTip(S32 x, S32 y, MASK mask) { - BOOL handled = handleMouseEvent(boost::bind(&LLMouseHandler::handleToolTip, _1, _2, _3, mask), view_visible, x, y, false); - if (!handled) - { - handled = LLPanel::handleToolTip(x, y, mask); - } - return handled; + BOOL handled = handleMouseEvent(boost::bind(&LLMouseHandler::handleToolTip, _1, _2, _3, mask), view_visible, x, y, false); + if (!handled) + { + handled = LLPanel::handleToolTip(x, y, mask); + } + return handled; } void LLPopupView::addPopup(LLView* popup) { - if (popup) - { - mPopups.remove(popup->getHandle()); - mPopups.push_front(popup->getHandle()); - } + if (popup) + { + mPopups.remove(popup->getHandle()); + mPopups.push_front(popup->getHandle()); + } } void LLPopupView::removePopup(LLView* popup) { - if (popup) - { - mPopups.remove(popup->getHandle()); - popup->onTopLost(); - } + if (popup) + { + mPopups.remove(popup->getHandle()); + popup->onTopLost(); + } } void LLPopupView::clearPopups() { - for (popup_list_t::iterator popup_it = mPopups.begin(); - popup_it != mPopups.end();) - { - LLView* popup = popup_it->get(); + for (popup_list_t::iterator popup_it = mPopups.begin(); + popup_it != mPopups.end();) + { + LLView* popup = popup_it->get(); - popup_list_t::iterator cur_popup_it = popup_it; - ++popup_it; + popup_list_t::iterator cur_popup_it = popup_it; + ++popup_it; - mPopups.erase(cur_popup_it); - popup->onTopLost(); - } + mPopups.erase(cur_popup_it); + popup->onTopLost(); + } } -- cgit v1.2.3