From dfa047c462421e1c4f23375a620fcc01b6da2a00 Mon Sep 17 00:00:00 2001 From: Albert Malewski Date: Thu, 27 Aug 2015 09:45:37 +0200 Subject: [PATCH] Fixed crash that occurs after clicking on 'History' button [Issue#] https://bugs.tizen.org/jira/browse/TT-98 [Problem] Browser crashes after clicking on 'History' button. [Cause] There was used normal pointer that pointed to the destroyed object. [Sollution] 1. Changed type of HistoryService::getHistoryItems() and HistoryService::getMostVisitedHistoryItems() functions that the return std::shared_ptr > >. 2. Added HistoryService::getHistoryItem function which returns HistoryItem by id. 3. Added HistoryService::getCurrentTab functions that returns currently observed web page's info. 4. Changed type of h_item pointer in MoreMenuUI.cpp from normal pointer to shared_ptr. 5. Added listItemDel function to avoid memory leaks when genlist is cleared. [Verify] Launch browser > More Menu > History > Obs History should not crash after clicking on 'History' button. Change-Id: I9140c1dedc061b2d014792aa14da92053e7aa27f --- services/HistoryService/HistoryService.cpp | 121 +++++++++++++++++------------ services/HistoryService/HistoryService.h | 6 +- services/HistoryUI/HistoryUI.cpp | 6 +- services/HistoryUI/HistoryUI.h | 4 +- services/MainUI/MainUI.cpp | 6 +- services/MainUI/MainUI.h | 4 +- services/MoreMenuUI/MoreMenuUI.cpp | 21 +++-- services/MoreMenuUI/MoreMenuUI.h | 3 +- services/SimpleUI/SimpleUI.cpp | 23 +++--- services/SimpleUI/SimpleUI.h | 2 +- 10 files changed, 114 insertions(+), 82 deletions(-) diff --git a/services/HistoryService/HistoryService.cpp b/services/HistoryService/HistoryService.cpp index f2ebbfb..2a9db4f 100644 --- a/services/HistoryService/HistoryService.cpp +++ b/services/HistoryService/HistoryService.cpp @@ -134,9 +134,9 @@ bool isDuplicate(const char* title) } -HistoryItemVector& HistoryService::getMostVisitedHistoryItems() +std::shared_ptr HistoryService::getMostVisitedHistoryItems() { - history_list.clear(); + std::shared_ptr ret_history_list(new HistoryItemVector); int *ids=nullptr; int count=-1; @@ -208,10 +208,10 @@ HistoryItemVector& HistoryService::getMostVisitedHistoryItems() memcpy(hi->imageData, (void*)history_info.thumbnail, history_info.thumbnail_length); history->setThumbnail(hi); - history_list.push_back(history); + ret_history_list->push_back(history); } free(ids); - return history_list; + return ret_history_list; } void HistoryService::addHistoryItem(std::shared_ptr his,std::shared_ptr thumbnail){ @@ -317,71 +317,94 @@ void HistoryService::clearURLHistory(const std::string & url) historyDeleted(url); } - -HistoryItemVector& HistoryService::getHistoryItems(int historyDepthInDays, int maxItems) +std::shared_ptr HistoryService::getHistoryItem(int * ids, int idNumber) { - history_list.clear(); + bp_history_offset offset = (BP_HISTORY_O_URL | BP_HISTORY_O_TITLE | BP_HISTORY_O_FAVICON | BP_HISTORY_O_DATE_CREATED); + bp_history_info_fmt history_info; + bp_history_adaptor_get_info(ids[idNumber], offset, &history_info); + + int date; + bp_history_adaptor_get_date_created(ids[idNumber], &date); + + struct tm *item_time_info; + time_t item_time = (time_t) date; + item_time_info = localtime(&item_time); + + int m_year = item_time_info->tm_year; + int m_month = item_time_info->tm_mon + 1; + int m_day = item_time_info->tm_yday; + int m_month_day = item_time_info->tm_mday; + int m_date = date; + int min = item_time_info->tm_min; + int hour = item_time_info->tm_hour; + int sec = item_time_info->tm_sec; + + m_year = 2000 + m_year % 100; + + std::shared_ptr history = std::make_shared (std::string(history_info.url)); + boost::gregorian::date d(m_year, m_month, m_month_day); + boost::posix_time::ptime t(d, boost::posix_time::time_duration(hour, min, sec)); + history->setLastVisit(t); + history->setUrl(std::string(history_info.url ? history_info.url : "")); + history->setTitle(std::string(history_info.title ? history_info.title : "")); + + //thumbail + std::shared_ptr hi = std::make_shared(); + hi->imageType = tizen_browser::tools::BrowserImage::ImageType::ImageTypePNG; + hi->width = history_info.thumbnail_width; + hi->height = history_info.thumbnail_height; + hi->dataSize = history_info.thumbnail_length; + hi->imageData = (void*) malloc(history_info.thumbnail_length); + memcpy(hi->imageData, (void*) history_info.thumbnail, history_info.thumbnail_length); + history->setThumbnail(hi); + + return history; +} +std::shared_ptr HistoryService::getCurrentTab() +{ int *ids=nullptr; - int count=-1; + int count = -1; bp_history_rows_cond_fmt conds; conds.limit = 20; //no of rows to get negative means no limitation conds.offset = -1; //the first row's index - conds.order_offset =BP_HISTORY_O_DATE_CREATED; // property to sort + conds.order_offset = BP_HISTORY_O_DATE_VISITED; // property to sort conds.ordering = 1; //way of ordering 0 asc 1 desc conds.period_offset = BP_HISTORY_O_DATE_CREATED; conds.period_type = BP_HISTORY_DATE_TODAY; - int ret = bp_history_adaptor_get_cond_ids_p(&ids ,&count, &conds, 0, nullptr, 0); + int ret = bp_history_adaptor_get_cond_ids_p(&ids , &count, &conds, 0, nullptr, 0); if (ret<0){ BROWSER_LOGD("Error! Could not get ids!"); } - bp_history_offset offset = (BP_HISTORY_O_URL | BP_HISTORY_O_TITLE | BP_HISTORY_O_FAVICON | BP_HISTORY_O_DATE_CREATED); - - for(int i = 0; i< count; i++){ - bp_history_info_fmt history_info; - bp_history_adaptor_get_info(ids[i],offset,&history_info); - - int date; - bp_history_adaptor_get_date_created(ids[i], &date); - - struct tm *item_time_info; - time_t item_time = (time_t)date; - item_time_info = localtime(&item_time); - - int m_year = item_time_info->tm_year; - int m_month = item_time_info->tm_mon + 1; - int m_day = item_time_info->tm_yday; - int m_month_day = item_time_info->tm_mday; - int m_date = date; - int min = item_time_info->tm_min; - int hour= item_time_info->tm_hour; - int sec = item_time_info->tm_sec; + return getHistoryItem(ids); +} - m_year = 2000 + m_year % 100; +std::shared_ptr HistoryService::getHistoryItems(bp_history_date_defs period, int maxItems) +{ + std::shared_ptr ret_history_list(new HistoryItemVector); - std::shared_ptr history = std::make_shared(std::string(history_info.url)); - boost::gregorian::date d(m_year,m_month,m_month_day); - boost::posix_time::ptime t(d,boost::posix_time::time_duration(hour,min,sec)); - history->setLastVisit(t); - history->setUrl(std::string(history_info.url ? history_info.url : "")); - history->setTitle(std::string(history_info.title ? history_info.title : "")); + int *ids=nullptr; + int count=-1; + bp_history_rows_cond_fmt conds; + conds.limit = 20; //no of rows to get negative means no limitation + conds.offset = -1; //the first row's index + conds.order_offset = BP_HISTORY_O_DATE_VISITED; // property to sort + conds.ordering = 1; //way of ordering 0 asc 1 desc + conds.period_offset = BP_HISTORY_O_DATE_CREATED; + conds.period_type = period; - //thumbail - std::shared_ptr hi = std::make_shared(); - hi->imageType = tizen_browser::tools::BrowserImage::ImageType::ImageTypePNG; - hi->width = history_info.thumbnail_width; - hi->height = history_info.thumbnail_height; - hi->dataSize = history_info.thumbnail_length; - hi->imageData = (void*)malloc(history_info.thumbnail_length); - memcpy(hi->imageData, (void*)history_info.thumbnail, history_info.thumbnail_length); - history->setThumbnail(hi); + int ret = bp_history_adaptor_get_cond_ids_p(&ids ,&count, &conds, 0, nullptr, 0); + if (ret<0) { + BROWSER_LOGD("Error! Could not get ids!"); + } - history_list.push_back(history); + for(int i = 0; i< count; i++) { + ret_history_list->push_back(getHistoryItem(ids, i)); } free(ids); - return history_list; + return ret_history_list; } int HistoryService::getHistoryVisitCounter(const std::string & url) diff --git a/services/HistoryService/HistoryService.h b/services/HistoryService/HistoryService.h index 3f2c44e..966a37b 100644 --- a/services/HistoryService/HistoryService.h +++ b/services/HistoryService/HistoryService.h @@ -76,8 +76,9 @@ public: /** * @throws HistoryException on error */ - HistoryItemVector & getHistoryItems(int historyDepthInDays = 7, int maxItems = 50); - HistoryItemVector & getMostVisitedHistoryItems(); + std::shared_ptr getHistoryItems(bp_history_date_defs period = BP_HISTORY_DATE_TODAY, int maxItems = 50); + std::shared_ptr getCurrentTab(); + std::shared_ptr getMostVisitedHistoryItems(); /** * @throws HistoryException on error @@ -106,6 +107,7 @@ private: */ void initDatabaseBookmark(const std::string & db_str); + std::shared_ptr getHistoryItem(int* ids, int idNumber = 0); std::shared_ptr getStorageManager(); }; diff --git a/services/HistoryUI/HistoryUI.cpp b/services/HistoryUI/HistoryUI.cpp index c4ec024..a2dc558 100644 --- a/services/HistoryUI/HistoryUI.cpp +++ b/services/HistoryUI/HistoryUI.cpp @@ -178,7 +178,7 @@ void HistoryUI::addItems() BROWSER_LOGD("%s:%d %s", __FILE__, __LINE__, __func__); } -void HistoryUI::addHistoryItem(std::shared_ptr hi) +void HistoryUI::addHistoryItem(std::shared_ptr hi) { BROWSER_LOGD("%s:%d %s", __FILE__, __LINE__, __func__); HistoryItemData *itemData = new HistoryItemData(); @@ -188,11 +188,11 @@ void HistoryUI::addHistoryItem(std::shared_ptr > items) +void HistoryUI::addHistoryItems(std::shared_ptr items) { BROWSER_LOGD("%s:%d %s", __FILE__, __LINE__, __func__); _history_item_data.clear(); - for (auto it = items.begin(); it != items.end(); ++it) + for (auto it = items->begin(); it != items->end(); ++it) addHistoryItem(*it); } diff --git a/services/HistoryUI/HistoryUI.h b/services/HistoryUI/HistoryUI.h index f6ac0b2..1eb4a24 100644 --- a/services/HistoryUI/HistoryUI.h +++ b/services/HistoryUI/HistoryUI.h @@ -38,8 +38,8 @@ public: ~HistoryUI(); void init(Evas_Object *main_layout); virtual std::string getName(); - void addHistoryItem(std::shared_ptr); - void addHistoryItems(std::vector >); + void addHistoryItem(std::shared_ptr); + void addHistoryItems(std::shared_ptr); void removeHistoryItem(const std::string& uri); void clearItems(); void hide(); diff --git a/services/MainUI/MainUI.cpp b/services/MainUI/MainUI.cpp index 6d86e66..e011d46 100644 --- a/services/MainUI/MainUI.cpp +++ b/services/MainUI/MainUI.cpp @@ -297,7 +297,7 @@ void MainUI::_bookmark_manager_clicked(void * data, Evas_Object * /* obj */, voi mainUI->bookmarkManagerClicked(std::string()); } -void MainUI::addHistoryItem(std::shared_ptr hi) +void MainUI::addHistoryItem(std::shared_ptr hi) { BROWSER_LOGD("%s:%d %s", __FILE__, __LINE__, __func__); if (m_map_history_views.size() >= MAX_TILES_NUMBER) @@ -323,11 +323,11 @@ void MainUI::addHistoryItem(std::shared_ptr(hi->getUrl(),historyView)); } -void MainUI::addHistoryItems(std::vector > items) +void MainUI::addHistoryItems(std::shared_ptr items) { BROWSER_LOGD("%s:%d %s", __FILE__, __LINE__, __func__); int i = 0; - for (auto it = items.begin(); it != items.end(); ++it) { + for (auto it = items->begin(); it != items->end(); ++it) { i++; if (i > MAX_TILES_NUMBER) break; diff --git a/services/MainUI/MainUI.h b/services/MainUI/MainUI.h index 35a3794..6809fd0 100644 --- a/services/MainUI/MainUI.h +++ b/services/MainUI/MainUI.h @@ -48,8 +48,8 @@ public: void showBookmarks(); void clearItems(); - void addHistoryItem(std::shared_ptr); - void addHistoryItems(std::vector >); + void addHistoryItem(std::shared_ptr); + void addHistoryItems(std::shared_ptr); void addBookmarkItem(std::shared_ptr); void addBookmarkItems(std::vector >); diff --git a/services/MoreMenuUI/MoreMenuUI.cpp b/services/MoreMenuUI/MoreMenuUI.cpp index d28c73c..ed361b6 100644 --- a/services/MoreMenuUI/MoreMenuUI.cpp +++ b/services/MoreMenuUI/MoreMenuUI.cpp @@ -34,7 +34,7 @@ EXPORT_SERVICE(MoreMenuUI, "org.tizen.browser.moremenuui") struct ItemData{ tizen_browser::base_ui::MoreMenuUI * m_moreMenu; - tizen_browser::services::HistoryItem * h_item; + std::shared_ptr h_item; Elm_Object_Item * e_item; }; @@ -98,7 +98,7 @@ void MoreMenuUI::show(Evas_Object* parent) addItems(); } -void MoreMenuUI::showCurrentTab(const std::shared_ptr item) +void MoreMenuUI::showCurrentTab(std::shared_ptr item) { BROWSER_LOGD("[%s:%d] ", __PRETTY_FUNCTION__, __LINE__); elm_theme_extension_add(NULL, m_edjFilePath.c_str()); @@ -119,11 +119,11 @@ void MoreMenuUI::showCurrentTab(const std::shared_ptrfunc.text_get = &listItemTextGet; m_itemClass->func.content_get = &listItemContentGet; m_itemClass->func.state_get = 0; - m_itemClass->func.del = 0; + m_itemClass->func.del = &listItemDel; ItemData * id = new ItemData; id->m_moreMenu = this; - id->h_item = item ? item.get() : NULL; + id->h_item = item; Elm_Object_Item* elmItem = elm_genlist_item_append(m_genList, //genlist m_itemClass, //item Class id, @@ -148,7 +148,7 @@ Evas_Object* MoreMenuUI::listItemContentGet(void* data, Evas_Object* obj, const const char *part_name3 = "close_click"; static const int part_name3_len = strlen(part_name3); - if (!strncmp(part_name1, part, part_name1_len) && id->h_item && id->h_item->getFavIcon()) { + if (!strncmp(part_name1, part, part_name1_len) && id->h_item.get() && id->h_item->getFavIcon()) { // Currently favicon is not getting fetched from the engine, // so we are showing Google's favicon by default. Evas_Object *thumb_nail = elm_icon_add(obj); @@ -259,14 +259,14 @@ char* MoreMenuUI::listItemTextGet(void* data, Evas_Object*, const char* part) static const int part_name2_len = strlen(part_name2); if (!strncmp(part_name1, part, part_name1_len)) { - if (!id->h_item) { + if (!id->h_item.get()) { return strdup("New Tab"); } return strdup(id->h_item->getTitle().c_str()); } if (!strncmp(part_name2, part, part_name2_len)) { - if(!id->h_item) + if(!id->h_item.get()) return strdup(""); return strdup(id->h_item->getUrl().c_str()); } @@ -274,6 +274,13 @@ char* MoreMenuUI::listItemTextGet(void* data, Evas_Object*, const char* part) return strdup(""); } +void MoreMenuUI::listItemDel(void* data, Evas_Object* obj) +{ + BROWSER_LOGD("[%s:%d] ", __PRETTY_FUNCTION__, __LINE__); + if(data) + delete (ItemData *)data; +} + void MoreMenuUI::hide() { evas_object_hide(elm_layout_content_get(m_mm_layout, "elm.swallow.grid")); diff --git a/services/MoreMenuUI/MoreMenuUI.h b/services/MoreMenuUI/MoreMenuUI.h index 103b8c7..9d0b4d4 100644 --- a/services/MoreMenuUI/MoreMenuUI.h +++ b/services/MoreMenuUI/MoreMenuUI.h @@ -63,7 +63,7 @@ public: MoreMenuUI(); ~MoreMenuUI(); void show(Evas_Object *main_layout); - void showCurrentTab(const std::shared_ptr item); + void showCurrentTab(std::shared_ptr item); virtual std::string getName(); void addItems(); void hide(); @@ -86,6 +86,7 @@ private: static Evas_Object* listItemContentGet(void *data, Evas_Object *obj, const char *part); static char* listItemTextGet(void *data, Evas_Object *obj, const char *part); + static void listItemDel(void *, Evas_Object *); void newFolderPopup(std::string); void NewFolderCreate(Evas_Object * popup_content); diff --git a/services/SimpleUI/SimpleUI.cpp b/services/SimpleUI/SimpleUI.cpp index f5f18f2..7d8e1fa 100644 --- a/services/SimpleUI/SimpleUI.cpp +++ b/services/SimpleUI/SimpleUI.cpp @@ -108,7 +108,7 @@ std::vector > SimpleUI::g return m_favoriteService->getBookmarks(folder_id); } -std::vector > SimpleUI::getHistory() +std::shared_ptr SimpleUI::getHistory() { return m_historyService->getMostVisitedHistoryItems(); } @@ -1139,25 +1139,24 @@ void SimpleUI::showMoreMenu() { BROWSER_LOGD("[%s:%d] ", __PRETTY_FUNCTION__, __LINE__); bool current_tab_as_new_tab = isHomePageActive() || (m_historyService->getHistoryItemsCount() == 0); - if(!m_moreMenuUI){ - m_moreMenuUI = - std::dynamic_pointer_cast - - (tizen_browser::core::ServiceManager::getInstance().getService("org.tizen.browser.moremenuui")); + if (!m_moreMenuUI) { + m_moreMenuUI = std::dynamic_pointer_cast + (tizen_browser::core::ServiceManager::getInstance().getService("org.tizen.browser.moremenuui")); M_ASSERT(m_moreMenuUI); + m_moreMenuUI->bookmarkManagerClicked.connect(boost::bind(&SimpleUI::onBookmarkManagerButtonClicked, this, _1)); - m_moreMenuUI->historyUIClicked.connect(boost::bind(&SimpleUI::showHistoryUI, this,_1)); - m_moreMenuUI->settingsClicked.connect(boost::bind(&SimpleUI::showSettingsUI, this,_1)); + m_moreMenuUI->historyUIClicked.connect(boost::bind(&SimpleUI::showHistoryUI, this, _1)); + m_moreMenuUI->settingsClicked.connect(boost::bind(&SimpleUI::showSettingsUI, this, _1)); m_moreMenuUI->closeMoreMenuClicked.disconnect_all_slots(); - m_moreMenuUI->closeMoreMenuClicked.connect(boost::bind(&SimpleUI::closeMoreMenu, this,_1)); + m_moreMenuUI->closeMoreMenuClicked.connect(boost::bind(&SimpleUI::closeMoreMenu, this, _1)); m_moreMenuUI->addToBookmarkClicked.disconnect_all_slots(); m_moreMenuUI->addToBookmarkClicked.connect(boost::bind(&SimpleUI::addBookmarkFolders, this)); m_moreMenuUI->AddBookmarkInput.disconnect_all_slots(); - m_moreMenuUI->AddBookmarkInput.connect(boost::bind(&SimpleUI::addToBookmarks, this,_1)); + m_moreMenuUI->AddBookmarkInput.connect(boost::bind(&SimpleUI::addToBookmarks, this, _1)); m_moreMenuUI->BookmarkFolderCreated.disconnect_all_slots(); - m_moreMenuUI->BookmarkFolderCreated.connect(boost::bind(&SimpleUI::newFolderMoreMenu, this,_1,_2)); + m_moreMenuUI->BookmarkFolderCreated.connect(boost::bind(&SimpleUI::newFolderMoreMenu, this, _1, _2)); m_moreMenuUI->show(m_window.get()); - m_moreMenuUI->showCurrentTab(current_tab_as_new_tab ? nullptr : m_historyService->getHistoryItems().front()); + m_moreMenuUI->showCurrentTab(current_tab_as_new_tab ? nullptr : m_historyService->getCurrentTab()); BROWSER_LOGD("[%s:%d] ", __PRETTY_FUNCTION__, __LINE__); } } diff --git a/services/SimpleUI/SimpleUI.h b/services/SimpleUI/SimpleUI.h index 06bbe47..e4783dd 100644 --- a/services/SimpleUI/SimpleUI.h +++ b/services/SimpleUI/SimpleUI.h @@ -122,7 +122,7 @@ private: void bookmarkCheck(); std::vector > getBookmarks(int folder_id = -1); std::vector > getBookmarkFolders(int folder_id); - std::vector > getHistory(); + std::shared_ptr getHistory(); void onBookmarkAdded(std::shared_ptr bookmarkItem); void onBookmarkClicked(std::shared_ptr bookmarkItem); -- 2.7.4