From e0f1f3b4d1860143070d15501e19546680ed664a Mon Sep 17 00:00:00 2001 From: Ethan Yonker Date: Tue, 27 Oct 2015 09:49:01 -0500 Subject: Move reloading of theme outside of the action thread Reloading the GUI while the GUI is running and rendering is very dangerous as we may be deleting GUI resources before or while we are trying to render those same items. This change will allow us to safely load new resources and delete the old ones. Fixed a double free situation that was causing a crash on every other theme reload. Clear parsed xml documents when we are done loading the theme instead of keeping them in memory for the life of the theme to help save a little memory. Store starting page so that if we request a reload, we can enter the theme at the same starting point, which may come in handy if we allow for language selection on alternate starting pages such as the decrypt prompt or the system read only prompt. Change-Id: I45a7e3fb3daeefac56d70f8d4936938eb1244b99 --- gui/action.cpp | 27 +++++-------------- gui/gui.cpp | 2 ++ gui/pages.cpp | 84 +++++++++++++++++++++++++++++++++++++++++++--------------- gui/pages.hpp | 11 ++++---- 4 files changed, 78 insertions(+), 46 deletions(-) diff --git a/gui/action.cpp b/gui/action.cpp index 86907a54f..c48c390f9 100644 --- a/gui/action.cpp +++ b/gui/action.cpp @@ -541,26 +541,13 @@ int GUIAction::page(std::string arg) int GUIAction::reload(std::string arg __unused) { - int check = 0, ret_val = 0; - std::string theme_path; - - theme_path = DataManager::GetSettingsStoragePath(); - if (PartitionManager.Mount_By_Path(theme_path.c_str(), 1) < 0) { - LOGERR("Unable to mount %s during reload function startup.\n", theme_path.c_str()); - check = 1; - } - - theme_path += "/TWRP/theme/ui.zip"; - if (check != 0 || PageManager::ReloadPackage("TWRP", theme_path) != 0) - { - // Loading the custom theme failed - try loading the stock theme - LOGINFO("Attempting to reload stock theme...\n"); - if (PageManager::ReloadPackage("TWRP", TWRES "ui.xml")) - { - LOGERR("Failed to load base packages.\n"); - ret_val = 1; - } - } + PageManager::RequestReload(); + // The actual reload is handled in pages.cpp in PageManager::RunReload() + // The reload will occur on the next Update or Render call and will + // be performed in the rendoer thread instead of the action thread + // to prevent crashing which could occur when we start deleting + // GUI resources in the action thread while we attempt to render + // with those same resources in another thread. return 0; } diff --git a/gui/gui.cpp b/gui/gui.cpp index a5ac33e86..31b61be6d 100644 --- a/gui/gui.cpp +++ b/gui/gui.cpp @@ -663,6 +663,8 @@ static int runPages(const char *page_name, const int stop_on_page_done) int ret = PageManager::Update(); if (ret == 0) ++idle_frames; + else if (ret == -2) + break; // Theme reload failure else idle_frames = 0; // due to possible animation objects, we need to delay activating the input timeout diff --git a/gui/pages.cpp b/gui/pages.cpp index 0a1de96f3..0511b1ab5 100644 --- a/gui/pages.cpp +++ b/gui/pages.cpp @@ -55,6 +55,8 @@ PageSet* PageManager::mCurrentSet; PageSet* PageManager::mBaseSet = NULL; MouseCursor *PageManager::mMouseCursor = NULL; HardwareKeyboard *PageManager::mHardwareKeyboard = NULL; +bool PageManager::mReloadTheme = false; +std::string PageManager::mStartPage = "main"; int tw_x_offset = 0; int tw_y_offset = 0; @@ -650,15 +652,12 @@ int Page::NotifyVarChange(std::string varName, std::string value) return 0; } -PageSet::PageSet(char* xmlFile) +PageSet::PageSet(const char* xmlFile) { mResources = new ResourceManager; mCurrentPage = NULL; - mXmlFile = xmlFile; - if (xmlFile) - mDoc.parse<0>(mXmlFile); - else + if (!xmlFile) mCurrentPage = new Page(NULL, NULL); } @@ -669,23 +668,17 @@ PageSet::~PageSet() delete *itr; delete mResources; - free(mXmlFile); - - mDoc.clear(); - - for (std::vector*>::iterator itr = mIncludedDocs.begin(); itr != mIncludedDocs.end(); ++itr) { - (*itr)->clear(); - delete *itr; - } } -int PageSet::Load(ZipArchive* package) +int PageSet::Load(ZipArchive* package, char* xmlFile) { + xml_document<> mDoc; xml_node<>* parent; xml_node<>* child; xml_node<>* xmltemplate; xml_node<>* xmlstyle; + mDoc.parse<0>(xmlFile); parent = mDoc.first_node("recovery"); if (!parent) parent = mDoc.first_node("install"); @@ -772,11 +765,15 @@ int PageSet::Load(ZipArchive* package) if (child) { if (LoadPages(child)) { LOGERR("PageSet::Load returning -1\n"); + mDoc.clear(); return -1; } } - return CheckInclude(package, &mDoc); + int ret = CheckInclude(package, &mDoc); + mDoc.clear(); + templates.clear(); + return ret; } int PageSet::CheckInclude(ZipArchive* package, xml_document<> *parentDoc) @@ -867,17 +864,19 @@ int PageSet::CheckInclude(ZipArchive* package, xml_document<> *parentDoc) return -1; } - mIncludedDocs.push_back(doc); - if (CheckInclude(package, doc)) { + doc->clear(); + delete doc; free(xmlFile); return -1; } + doc->clear(); + delete doc; + free(xmlFile); chld = chld->next_sibling("xmlfile"); } - if (xmlFile) - free(xmlFile); + return 0; } @@ -1192,6 +1191,9 @@ int PageManager::LoadPackage(std::string name, std::string package, std::string int ret; MemMapping map; + mReloadTheme = false; + mStartPage = startpage; + // Open the XML file LOGINFO("Loading package: %s (%s)\n", name.c_str(), package.c_str()); if (package.size() > 4 && package.substr(package.size() - 4) != ".zip") @@ -1229,7 +1231,7 @@ int PageManager::LoadPackage(std::string name, std::string package, std::string pageSet = mCurrentSet; mCurrentSet = new PageSet(xmlFile); - ret = mCurrentSet->Load(pZip); + ret = mCurrentSet->Load(pZip, xmlFile); if (ret == 0) { mCurrentSet->SetPage(startpage); @@ -1297,6 +1299,8 @@ int PageManager::ReloadPackage(std::string name, std::string package) { std::map::iterator iter; + mReloadTheme = false; + iter = mPageSets.find(name); if (iter == mPageSets.end()) return -1; @@ -1307,7 +1311,7 @@ int PageManager::ReloadPackage(std::string name, std::string package) PageSet* set = (*iter).second; mPageSets.erase(iter); - if (LoadPackage(name, package, "main") != 0) + if (LoadPackage(name, package, mStartPage) != 0) { LOGERR("Failed to load package '%s'.\n", package.c_str()); mPageSets.insert(std::pair(name, set)); @@ -1335,6 +1339,38 @@ void PageManager::ReleasePackage(std::string name) return; } +int PageManager::RunReload() { + int ret_val = 0; + std::string theme_path; + + if (!mReloadTheme) + return 0; + + mReloadTheme = false; + theme_path = DataManager::GetSettingsStoragePath(); + if (PartitionManager.Mount_By_Path(theme_path.c_str(), 1) < 0) { + LOGERR("Unable to mount %s during gui_reload_theme function.\n", theme_path.c_str()); + ret_val = 1; + } + + theme_path += "/TWRP/theme/ui.zip"; + if (ret_val != 0 || ReloadPackage("TWRP", theme_path) != 0) + { + // Loading the custom theme failed - try loading the stock theme + LOGINFO("Attempting to reload stock theme...\n"); + if (ReloadPackage("TWRP", TWRES "ui.xml")) + { + LOGERR("Failed to load base packages.\n"); + ret_val = 1; + } + } + return ret_val; +} + +void PageManager::RequestReload() { + mReloadTheme = true; +} + int PageManager::ChangePage(std::string name) { DataManager::SetValue("tw_operation_state", 0); @@ -1383,6 +1419,9 @@ int PageManager::IsCurrentPage(Page* page) int PageManager::Render(void) { + if(blankTimer.isScreenOff()) + return 0; + int res = (mCurrentSet ? mCurrentSet->Render() : -1); if(mMouseCursor) mMouseCursor->Render(); @@ -1433,6 +1472,9 @@ int PageManager::Update(void) if(blankTimer.isScreenOff()) return 0; + if (RunReload()) + return -2; + int res = (mCurrentSet ? mCurrentSet->Update() : -1); if(mMouseCursor) diff --git a/gui/pages.hpp b/gui/pages.hpp index 03fb0e5de..018c2cab2 100644 --- a/gui/pages.hpp +++ b/gui/pages.hpp @@ -78,11 +78,11 @@ protected: class PageSet { public: - PageSet(char* xmlFile); + PageSet(const char* xmlFile); virtual ~PageSet(); public: - int Load(ZipArchive* package); + int Load(ZipArchive* package, char* xmlFile); int CheckInclude(ZipArchive* package, xml_document<> *parentDoc); Page* FindPage(std::string name); @@ -109,14 +109,11 @@ protected: int LoadVariables(xml_node<>* vars); protected: - char* mXmlFile; - xml_document<> mDoc; ResourceManager* mResources; std::vector mPages; std::vector*> templates; Page* mCurrentPage; std::vector mOverlays; // Special case for popup dialogs and the lock screen - std::vector*> mIncludedDocs; }; class PageManager @@ -128,6 +125,8 @@ public: static PageSet* SelectPackage(std::string name); static int ReloadPackage(std::string name, std::string package); static void ReleasePackage(std::string name); + static int RunReload(); + static void RequestReload(); // Used for actions and pages static int ChangePage(std::string name); @@ -166,6 +165,8 @@ protected: static PageSet* mBaseSet; static MouseCursor *mMouseCursor; static HardwareKeyboard *mHardwareKeyboard; + static bool mReloadTheme; + static std::string mStartPage; }; #endif // _PAGES_HEADER_HPP -- cgit v1.2.3