[webkit-reviews] review granted: [Bug 24259] [Gtk] get the HTTP layout tests going : [Attachment 28130] history item fixes and API additions for dumping a history item

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Mar 1 08:07:07 PST 2009


Holger Freyther <zecke at selfish.org> has granted Jan Alonzo
<jmalonzo at gmail.com>'s request for review:
Bug 24259: [Gtk] get the HTTP layout tests going
https://bugs.webkit.org/show_bug.cgi?id=24259

Attachment 28130: history item fixes and API additions for dumping a history
item
https://bugs.webkit.org/attachment.cgi?id=28130&action=review

------- Additional Comments from Holger Freyther <zecke at selfish.org>

> -    priv->historyItem = WebCore::HistoryItem::create();
> -    webkit_history_item_add(webHistoryItem, priv->historyItem.get());
> +    RefPtr<WebCore::HistoryItem> item = WebCore::HistoryItem::create();
> +    priv->historyItem = item.get();
> +    webkit_history_item_add(webHistoryItem, priv->historyItem);

make that a item.release()?



>      return webHistoryItem;
>  }
> @@ -323,8 +312,9 @@
>      WebKitWebHistoryItem* webHistoryItem =
WEBKIT_WEB_HISTORY_ITEM(g_object_new(WEBKIT_TYPE_WEB_HISTORY_ITEM, NULL));
>      WebKitWebHistoryItemPrivate* priv = webHistoryItem->priv;
>  
> -    priv->historyItem = WebCore::HistoryItem::create(historyUri,
historyTitle, 0);
> -    webkit_history_item_add(webHistoryItem, priv->historyItem.get());
> +    RefPtr<WebCore::HistoryItem> item =
WebCore::HistoryItem::create(historyUri, historyTitle, 0);
> +    priv->historyItem = item.get();
> +    webkit_history_item_add(webHistoryItem, priv->historyItem);


another release()? I'm a bit scared with the change to PassRefPtr... but let me
take another look.



> +    WebCore::CString t = item->target().utf8();
> +    return t.data();

strdup? the printf() in the DRT might be a bit dangerous.


okay, the other bits look fine. before landing please coordinate with xan/kov
to not conflict with their release work.


More information about the webkit-reviews mailing list