From 8fb2fc6fc347ddd68db0ce70648e9a2d25ef88e3 Mon Sep 17 00:00:00 2001 From: Vincent Sanders Date: Sun, 10 Sep 2017 16:05:41 +0100 Subject: rationalise history icon bitmap handling to ensure correct lifetime --- content/urldb.c | 47 ----------- content/urldb.h | 10 --- desktop/browser_history.c | 198 ++++++++++++++++++++++++++++++---------------- desktop/browser_history.h | 25 +++--- frontends/amiga/gui.c | 7 +- include/netsurf/bitmap.h | 2 +- include/netsurf/url_db.h | 9 --- 7 files changed, 147 insertions(+), 151 deletions(-) diff --git a/content/urldb.c b/content/urldb.c index add2a1d80..cacc475f2 100644 --- a/content/urldb.c +++ b/content/urldb.c @@ -202,7 +202,6 @@ struct path_data { char **fragment; /**< Array of fragments */ bool persistent; /**< This entry should persist */ - struct bitmap *thumb; /**< Thumbnail image of resource */ struct url_internal_data urld; /**< URL data for resource */ /** @@ -2702,10 +2701,6 @@ static void urldb_destroy_path_node_content(struct path_data *node) free(node->fragment[i]); free(node->fragment); - if (node->thumb) { - guit->bitmap->destroy(node->thumb); - } - free(node->urld.title); for (a = node->cookies; a; a = b) { @@ -3465,48 +3460,6 @@ bool urldb_get_cert_permissions(nsurl *url) } -/* exported interface documented in content/urldb.h */ -bool urldb_set_thumbnail(nsurl *url, struct bitmap *bitmap) -{ - struct path_data *p; - - assert(url); - - /* add url, in case it's missing */ - urldb_add_url(url); - - p = urldb_find_url(url); - if (p == NULL) { - return false; - } - - NSLOG(netsurf, INFO, "Setting bitmap on %s", nsurl_access(url)); - - if ((p->thumb) && (p->thumb != bitmap)) { - guit->bitmap->destroy(p->thumb); - } - - p->thumb = bitmap; - - return true; -} - - -/* exported interface documented in netsurf/url_db.h */ -struct bitmap *urldb_get_thumbnail(nsurl *url) -{ - struct path_data *p; - - assert(url); - - p = urldb_find_url(url); - if (!p) - return NULL; - - return p->thumb; -} - - /* exported interface documented in netsurf/url_db.h */ void urldb_iterate_partial(const char *prefix, diff --git a/content/urldb.h b/content/urldb.h index b2c233194..4aa548704 100644 --- a/content/urldb.h +++ b/content/urldb.h @@ -110,16 +110,6 @@ struct nsurl *urldb_get_url(struct nsurl *url); bool urldb_get_cert_permissions(struct nsurl *url); -/** - * Set thumbnail for url, replacing any existing thumbnail - * - * \param url Absolute URL to consider - * \param bitmap Opaque pointer to thumbnail data, or NULL to invalidate - * \return true on successful setting else false - */ -bool urldb_set_thumbnail(struct nsurl *url, struct bitmap *bitmap); - - /** * Parse Set-Cookie header and insert cookie(s) into database * diff --git a/desktop/browser_history.c b/desktop/browser_history.c index ba122f30d..c8a896290 100644 --- a/desktop/browser_history.c +++ b/desktop/browser_history.c @@ -50,7 +50,7 @@ * Clone a history entry * * \param history opaque history structure, as returned by history_create() - * \param entry entry to clone + * \param entry entry to clone * \return A cloned history entry or NULL on error */ static struct history_entry * @@ -62,47 +62,85 @@ browser_window_history__clone_entry(struct history *history, struct history_entry *prev = NULL; struct history_entry *new_entry; + assert(entry); assert(entry->page.url); assert(entry->page.title); /* clone the entry */ - new_entry = malloc(sizeof *entry); - if (!new_entry) + new_entry = calloc(1, sizeof *entry); + if (!new_entry) { return NULL; + } - memcpy(new_entry, entry, sizeof *entry); - new_entry->page.url = nsurl_ref(entry->page.url); - if (entry->page.frag_id) - new_entry->page.frag_id = lwc_string_ref(entry->page.frag_id); - + /* copy page information */ new_entry->page.title = strdup(entry->page.title); - if (!new_entry->page.url || !new_entry->page.title || - ((entry->page.frag_id) && (!new_entry->page.frag_id))) { - nsurl_unref(new_entry->page.url); - if (new_entry->page.frag_id) - lwc_string_unref(new_entry->page.frag_id); + if (new_entry->page.title == NULL) { + free(new_entry); + return NULL; + } + + new_entry->page.url = nsurl_ref(entry->page.url); + if (new_entry->page.url == NULL) { free(new_entry->page.title); free(new_entry); return NULL; } - /* update references */ - if (history->current == entry) - history->current = new_entry; + if (entry->page.frag_id == NULL) { + new_entry->page.frag_id = NULL; + } else { + new_entry->page.frag_id = lwc_string_ref(entry->page.frag_id); + if (new_entry->page.frag_id == NULL) { + nsurl_unref(new_entry->page.url); + free(new_entry); + return NULL; + } + } + + if (entry->page.bitmap == NULL) { + new_entry->page.bitmap = NULL; + } else { + /* create a new bitmap and copy original into it */ + unsigned char *bmsrc_data; + unsigned char *bmdst_data; + size_t bmsize; + + new_entry->page.bitmap = guit->bitmap->create(WIDTH, HEIGHT, + BITMAP_NEW | BITMAP_OPAQUE); + + if (new_entry->page.bitmap != NULL) { + bmsrc_data = guit->bitmap->get_buffer(entry->page.bitmap); + bmdst_data = guit->bitmap->get_buffer(new_entry->page.bitmap); + bmsize = guit->bitmap->get_rowstride(new_entry->page.bitmap) * + guit->bitmap->get_height(new_entry->page.bitmap); + memcpy(bmdst_data, bmsrc_data, bmsize); + } + } + + /* copy tree values */ + new_entry->back = entry->back; + new_entry->next = entry->next; + new_entry->forward = entry->forward; + new_entry->forward_pref = entry->forward_pref; + new_entry->forward_last = entry->forward_last; /* recurse for all children */ - for (child = new_entry->forward; child; child = child->next) { + for (child = new_entry->forward; child != NULL; child = child->next) { new_child = browser_window_history__clone_entry(history, child); - if (new_child) { - new_child->back = new_entry; - } else { + if (new_child == NULL) { nsurl_unref(new_entry->page.url); - if (new_entry->page.frag_id) + if (new_entry->page.frag_id) { lwc_string_unref(new_entry->page.frag_id); + } free(new_entry->page.title); + if (entry->page.bitmap != NULL) { + guit->bitmap->destroy(entry->page.bitmap); + } free(new_entry); return NULL; } + + new_child->back = new_entry; if (prev) prev->next = new_child; if (new_entry->forward == child) @@ -113,6 +151,12 @@ browser_window_history__clone_entry(struct history *history, new_entry->forward_last = new_child; prev = new_child; } + + /* update references */ + if (history->current == entry) { + history->current = new_entry; + } + return new_entry; } @@ -123,15 +167,20 @@ browser_window_history__clone_entry(struct history *history, static void browser_window_history__free_entry(struct history_entry *entry) { - if (!entry) - return; - browser_window_history__free_entry(entry->forward); - browser_window_history__free_entry(entry->next); - nsurl_unref(entry->page.url); - if (entry->page.frag_id) - lwc_string_unref(entry->page.frag_id); - free(entry->page.title); - free(entry); + if (entry != NULL) { + browser_window_history__free_entry(entry->forward); + browser_window_history__free_entry(entry->next); + + nsurl_unref(entry->page.url); + if (entry->page.frag_id) { + lwc_string_unref(entry->page.frag_id); + } + free(entry->page.title); + if (entry->page.bitmap != NULL) { + guit->bitmap->destroy(entry->page.bitmap); + } + free(entry); + } } @@ -297,14 +346,14 @@ nserror browser_window_history_clone(const struct browser_window *existing, /* exported interface documented in desktop/browser_history.h */ -nserror browser_window_history_add(struct browser_window *bw, - struct hlcache_handle *content, lwc_string *frag_id) +nserror +browser_window_history_add(struct browser_window *bw, + struct hlcache_handle *content, + lwc_string *frag_id) { struct history *history; struct history_entry *entry; - nsurl *nsurl = hlcache_handle_get_url(content); char *title; - struct bitmap *bitmap; nserror ret; assert(bw); @@ -313,26 +362,40 @@ nserror browser_window_history_add(struct browser_window *bw, history = bw->history; - /* allocate space */ entry = malloc(sizeof *entry); if (entry == NULL) { return NSERROR_NOMEM; } + /* page information */ title = strdup(content_get_title(content)); if (title == NULL) { free(entry); return NSERROR_NOMEM; } - entry->page.url = nsurl_ref(nsurl); + entry->page.url = nsurl_ref(hlcache_handle_get_url(content)); entry->page.frag_id = frag_id ? lwc_string_ref(frag_id) : NULL; entry->page.title = title; - entry->page.bitmap = NULL; + /* create thumbnail for localhistory view */ + NSLOG(netsurf, DEBUG, + "Creating thumbnail for %s", nsurl_access(entry->page.url)); + + entry->page.bitmap = guit->bitmap->create(WIDTH, HEIGHT, + BITMAP_NEW | BITMAP_CLEAR_MEMORY | BITMAP_OPAQUE); + if (entry->page.bitmap != NULL) { + ret = guit->bitmap->render(entry->page.bitmap, content); + if (ret != NSERROR_OK) { + /* Thumbnail render failed */ + NSLOG(netsurf, WARNING, "Thumbnail render failed"); + } + } + + /* insert into tree */ entry->back = history->current; - entry->next = 0; - entry->forward = entry->forward_pref = entry->forward_last = 0; + entry->next = NULL; + entry->forward = entry->forward_pref = entry->forward_last = NULL; entry->children = 0; if (history->current) { @@ -349,34 +412,6 @@ nserror browser_window_history_add(struct browser_window *bw, } history->current = entry; - /* if we have a thumbnail, don't update until the page has finished - * loading */ - bitmap = urldb_get_thumbnail(nsurl); - if (bitmap == NULL) { - NSLOG(netsurf, INFO, "Creating thumbnail for %s", - nsurl_access(nsurl)); - bitmap = guit->bitmap->create(WIDTH, HEIGHT, - BITMAP_NEW | BITMAP_CLEAR_MEMORY | - BITMAP_OPAQUE); - if (bitmap != NULL) { - ret = guit->bitmap->render(bitmap, content); - if (ret == NSERROR_OK) { - /* Successful thumbnail so register it - * with the url. - */ - urldb_set_thumbnail(nsurl, bitmap); - } else { - /* Thumbnailing failed. Ignore it - * silently but clean up bitmap. - */ - NSLOG(netsurf, INFO, "Thumbnail renderfailed"); - guit->bitmap->destroy(bitmap); - bitmap = NULL; - } - } - } - entry->page.bitmap = bitmap; - browser_window_history__layout(history); return NSERROR_OK; @@ -411,7 +446,9 @@ nserror browser_window_history_update(struct browser_window *bw, free(history->current->page.title); history->current->page.title = title; - guit->bitmap->render(history->current->page.bitmap, content); + if (history->current->page.bitmap != NULL) { + guit->bitmap->render(history->current->page.bitmap, content); + } return NSERROR_OK; } @@ -475,6 +512,27 @@ bool browser_window_history_forward_available(struct browser_window *bw) bw->history->current->forward_pref); } +/* exported interface documented in desktop/browser_history.h */ +nserror +browser_window_history_get_thumbnail(struct browser_window *bw, + struct bitmap **bitmap_out) +{ + struct bitmap *bitmap; + + if (!bw || !bw->history || !bw->history->current) { + return NSERROR_INVALID; + } + + if (bw->history->current->page.bitmap == NULL) { + bitmap = content_get_bitmap(bw->current_content); + } else { + bitmap = bw->history->current->page.bitmap; + } + + *bitmap_out = bitmap; + + return NSERROR_OK; +} /* exported interface documented in desktop/browser_history.h */ nserror browser_window_history_go(struct browser_window *bw, @@ -519,7 +577,7 @@ nserror browser_window_history_go(struct browser_window *bw, /* exported interface documented in desktop/browser_history.h */ -void browser_window_history_enumerate_forward(const struct browser_window *bw, +void browser_window_history_enumerate_forward(const struct browser_window *bw, browser_window_history_enumerate_cb cb, void *user_data) { struct history_entry *e; @@ -537,7 +595,7 @@ void browser_window_history_enumerate_forward(const struct browser_window *bw, /* exported interface documented in desktop/browser_history.h */ -void browser_window_history_enumerate_back(const struct browser_window *bw, +void browser_window_history_enumerate_back(const struct browser_window *bw, browser_window_history_enumerate_cb cb, void *user_data) { struct history_entry *e; diff --git a/desktop/browser_history.h b/desktop/browser_history.h index 9140e2ce0..d2021198c 100644 --- a/desktop/browser_history.h +++ b/desktop/browser_history.h @@ -75,6 +75,14 @@ bool browser_window_history_back_available(struct browser_window *bw); */ bool browser_window_history_forward_available(struct browser_window *bw); +/** + * Get the thumbnail bitmap for the current history entry + * + * \param bw The browser window + * \param bitmap The bitmat for the current history entry. + * \return NSERROR_OK or error code on faliure. + */ +nserror browser_window_history_get_thumbnail(struct browser_window *bw, struct bitmap **bitmap_out); /** * Callback function type for history enumeration @@ -136,21 +144,19 @@ struct nsurl *browser_window_history_entry_get_url(const struct history_entry *e /** * Returns the URL to a history entry * - * \param entry the history entry to retrieve the fragment id from - * \return the fragment id + * \param entry the history entry to retrieve the fragment id from + * \return the fragment id */ -const char *browser_window_history_entry_get_fragment_id( - const struct history_entry *entry); +const char *browser_window_history_entry_get_fragment_id(const struct history_entry *entry); /** * Returns the title of a history entry * - * \param entry the history entry to retrieve the title from - * \return the title + * \param entry The history entry to retrieve the title from + * \return the title */ -const char *browser_window_history_entry_get_title( - const struct history_entry *entry); +const char *browser_window_history_entry_get_title(const struct history_entry *entry); /** @@ -161,7 +167,6 @@ const char *browser_window_history_entry_get_title( * \param new_window open entry in new window * \return NSERROR_OK or error code on faliure. */ -nserror browser_window_history_go(struct browser_window *bw, - struct history_entry *entry, bool new_window); +nserror browser_window_history_go(struct browser_window *bw, struct history_entry *entry, bool new_window); #endif diff --git a/frontends/amiga/gui.c b/frontends/amiga/gui.c index fbaa2acfb..eca5b927c 100644 --- a/frontends/amiga/gui.c +++ b/frontends/amiga/gui.c @@ -2527,10 +2527,9 @@ static BOOL ami_gui_event(void *w) #ifdef __amigaos4__ case WMHI_ICONIFY: { - struct bitmap *bm; - - bm = urldb_get_thumbnail(browser_window_get_url(gwin->gw->bw)); - if(!bm) bm = content_get_bitmap(browser_window_get_content(gwin->gw->bw)); + struct bitmap *bm = NULL; + browser_window_history_get_thumbnail(gwin->gw->bw, + &bm); gwin->dobj = amiga_icon_from_bitmap(bm); amiga_icon_superimpose_favicon_internal(gwin->gw->favicon, gwin->dobj); diff --git a/include/netsurf/bitmap.h b/include/netsurf/bitmap.h index e54bdff85..a85efce99 100644 --- a/include/netsurf/bitmap.h +++ b/include/netsurf/bitmap.h @@ -154,7 +154,7 @@ struct gui_bitmap_table { * * \param bitmap The bitmap to save * \param path The path to save the bitmap to. - * \param flags Flags affectin the save. + * \param flags Flags affecting the save. */ bool (*save)(void *bitmap, const char *path, unsigned flags); diff --git a/include/netsurf/url_db.h b/include/netsurf/url_db.h index be2c656ff..217cf8fcd 100644 --- a/include/netsurf/url_db.h +++ b/include/netsurf/url_db.h @@ -95,15 +95,6 @@ void urldb_iterate_partial(const char *prefix, bool (*callback)(struct nsurl *ur void urldb_iterate_entries(bool (*callback)(struct nsurl *url, const struct url_data *data)); -/** - * Retrieve thumbnail data for given URL - * - * \param url Absolute URL to search for - * \return Pointer to thumbnail data, or NULL if not found. - */ -struct bitmap *urldb_get_thumbnail(struct nsurl *url); - - /** * Find data for an URL. * -- cgit v1.2.3