Fixed crash that occurs after clicking on 'History' button 98/46798/6 accepted/tizen/tv/20150828.012542 submit/tizen_tv/20150827.234458
authorAlbert Malewski <a.malewski@samsung.com>
Thu, 27 Aug 2015 07:45:37 +0000 (09:45 +0200)
committerAlbert Malewski <a.malewski@samsung.com>
Thu, 27 Aug 2015 10:43:59 +0000 (12:43 +0200)
[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<std::vector<std::shared_ptr<HistoryItem> > >.
            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
services/HistoryService/HistoryService.h
services/HistoryUI/HistoryUI.cpp
services/HistoryUI/HistoryUI.h
services/MainUI/MainUI.cpp
services/MainUI/MainUI.h
services/MoreMenuUI/MoreMenuUI.cpp
services/MoreMenuUI/MoreMenuUI.h
services/SimpleUI/SimpleUI.cpp
services/SimpleUI/SimpleUI.h

index f2ebbfb..2a9db4f 100644 (file)
@@ -134,9 +134,9 @@ bool isDuplicate(const char* title)
 }
 
 
-HistoryItemVector& HistoryService::getMostVisitedHistoryItems()
+std::shared_ptr<HistoryItemVector> HistoryService::getMostVisitedHistoryItems()
 {
-    history_list.clear();
+    std::shared_ptr<HistoryItemVector> 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<HistoryItem> his,std::shared_ptr<tizen_browser::tools::BrowserImage> thumbnail){
@@ -317,71 +317,94 @@ void HistoryService::clearURLHistory(const std::string & url)
     historyDeleted(url);
 }
 
-
-HistoryItemVector& HistoryService::getHistoryItems(int historyDepthInDays, int maxItems)
+std::shared_ptr<HistoryItem> 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<HistoryItem> history = std::make_shared <HistoryItem> (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<tizen_browser::tools::BrowserImage> hi = std::make_shared<tizen_browser::tools::BrowserImage>();
+    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<HistoryItem> 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<HistoryItemVector> HistoryService::getHistoryItems(bp_history_date_defs period, int maxItems)
+{
+    std::shared_ptr<HistoryItemVector> ret_history_list(new HistoryItemVector);
 
-        std::shared_ptr<HistoryItem> history = std::make_shared<HistoryItem>(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<tizen_browser::tools::BrowserImage> hi = std::make_shared<tizen_browser::tools::BrowserImage>();
-        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)
index 3f2c44e..966a37b 100644 (file)
@@ -76,8 +76,9 @@ public:
     /**
      * @throws HistoryException on error
      */
-    HistoryItemVector & getHistoryItems(int historyDepthInDays = 7, int maxItems = 50);
-    HistoryItemVector & getMostVisitedHistoryItems();
+    std::shared_ptr<HistoryItemVector> getHistoryItems(bp_history_date_defs period = BP_HISTORY_DATE_TODAY, int maxItems = 50);
+    std::shared_ptr<HistoryItem> getCurrentTab();
+    std::shared_ptr<HistoryItemVector> getMostVisitedHistoryItems();
 
     /**
      * @throws HistoryException on error
@@ -106,6 +107,7 @@ private:
      */
     void initDatabaseBookmark(const std::string & db_str);
 
+    std::shared_ptr<HistoryItem> getHistoryItem(int* ids, int idNumber = 0);
     std::shared_ptr<tizen_browser::services::StorageService> getStorageManager();
 
 };
index c4ec024..a2dc558 100644 (file)
@@ -178,7 +178,7 @@ void HistoryUI::addItems()
     BROWSER_LOGD("%s:%d %s", __FILE__, __LINE__, __func__);
 }
 
-void HistoryUI::addHistoryItem(std::shared_ptr<tizen_browser::services::HistoryItem> hi)
+void HistoryUI::addHistoryItem(std::shared_ptr<services::HistoryItem> hi)
 {
     BROWSER_LOGD("%s:%d %s", __FILE__, __LINE__, __func__);
     HistoryItemData *itemData = new HistoryItemData();
@@ -188,11 +188,11 @@ void HistoryUI::addHistoryItem(std::shared_ptr<tizen_browser::services::HistoryI
     setEmptyGengrid(false);
 }
 
-void HistoryUI::addHistoryItems(std::vector<std::shared_ptr<tizen_browser::services::HistoryItem> > items)
+void HistoryUI::addHistoryItems(std::shared_ptr<services::HistoryItemVector> 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);
 }
 
index f6ac0b2..1eb4a24 100644 (file)
@@ -38,8 +38,8 @@ public:
     ~HistoryUI();
     void init(Evas_Object *main_layout);
     virtual std::string getName();
-    void addHistoryItem(std::shared_ptr<tizen_browser::services::HistoryItem>);
-    void addHistoryItems(std::vector<std::shared_ptr<tizen_browser::services::HistoryItem> >);
+    void addHistoryItem(std::shared_ptr<services::HistoryItem>);
+    void addHistoryItems(std::shared_ptr<services::HistoryItemVector>);
     void removeHistoryItem(const std::string& uri);
     void clearItems();
     void hide();
index 6d86e66..e011d46 100644 (file)
@@ -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<tizen_browser::services::HistoryItem> hi)
+void MainUI::addHistoryItem(std::shared_ptr<services::HistoryItem> 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<tizen_browser::services::HistoryItem
     m_map_history_views.insert(std::pair<std::string,Elm_Object_Item*>(hi->getUrl(),historyView));
 }
 
-void MainUI::addHistoryItems(std::vector<std::shared_ptr<tizen_browser::services::HistoryItem> > items)
+void MainUI::addHistoryItems(std::shared_ptr<services::HistoryItemVector> 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;
index 35a3794..6809fd0 100644 (file)
@@ -48,8 +48,8 @@ public:
     void showBookmarks();
     void clearItems();
 
-    void addHistoryItem(std::shared_ptr<tizen_browser::services::HistoryItem>);
-    void addHistoryItems(std::vector<std::shared_ptr<tizen_browser::services::HistoryItem> >);
+    void addHistoryItem(std::shared_ptr<services::HistoryItem>);
+    void addHistoryItems(std::shared_ptr<services::HistoryItemVector>);
     void addBookmarkItem(std::shared_ptr<tizen_browser::services::BookmarkItem>);
     void addBookmarkItems(std::vector<std::shared_ptr<tizen_browser::services::BookmarkItem> >);
 
index d28c73c..ed361b6 100644 (file)
@@ -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<tizen_browser::services::HistoryItem> 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<tizen_browser::services::HistoryItem> item)
+void MoreMenuUI::showCurrentTab(std::shared_ptr<tizen_browser::services::HistoryItem> 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_ptr<tizen_browser::services::H
     m_itemClass->func.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"));
index 103b8c7..9d0b4d4 100644 (file)
@@ -63,7 +63,7 @@ public:
     MoreMenuUI();
     ~MoreMenuUI();
     void show(Evas_Object *main_layout);
-    void showCurrentTab(const std::shared_ptr<tizen_browser::services::HistoryItem> item);
+    void showCurrentTab(std::shared_ptr<tizen_browser::services::HistoryItem> 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);
index f5f18f2..7d8e1fa 100644 (file)
@@ -108,7 +108,7 @@ std::vector<std::shared_ptr<tizen_browser::services::BookmarkItem> > SimpleUI::g
     return m_favoriteService->getBookmarks(folder_id);
 }
 
-std::vector<std::shared_ptr<tizen_browser::services::HistoryItem> > SimpleUI::getHistory()
+std::shared_ptr<services::HistoryItemVector> 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::base_ui::MoreMenuUI,tizen_browser::core::AbstractService>
-                (tizen_browser::core::ServiceManager::getInstance().getService("org.tizen.browser.moremenuui"));
+    if (!m_moreMenuUI) {
+        m_moreMenuUI = std::dynamic_pointer_cast<tizen_browser::base_ui::MoreMenuUI, tizen_browser::core::AbstractService>
+                       (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__);
     }
 }
index 06bbe47..e4783dd 100644 (file)
@@ -122,7 +122,7 @@ private:
     void bookmarkCheck();
     std::vector<std::shared_ptr<tizen_browser::services::BookmarkItem> > getBookmarks(int folder_id = -1);
     std::vector<std::shared_ptr<tizen_browser::services::BookmarkItem> > getBookmarkFolders(int folder_id);
-    std::vector<std::shared_ptr<tizen_browser::services::HistoryItem> > getHistory();
+    std::shared_ptr<services::HistoryItemVector> getHistory();
     void onBookmarkAdded(std::shared_ptr<tizen_browser::services::BookmarkItem> bookmarkItem);
 
     void onBookmarkClicked(std::shared_ptr<tizen_browser::services::BookmarkItem> bookmarkItem);