Bug 24259 – [Gtk] get the HTTP layout tests going

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Jan Alonzo]

We should get the HTTP tests working so we can test navigation, xhr, etc...

[Jan Alonzo]

Created attachment 28113 [details]
added necessary API additions and fixes for dumping the back/forward list, etc..

This patch adds support for dumping the back forward list history items, dumping plain/text as text, and history item API additions (currently private) and fixes to get the dumping of history items working.

Currently 111 tests are passing. All failed tests or untested folders are currently skipped.

[Jan Alonzo]

Created attachment 28130 [details]
history item fixes and API additions for dumping a history item

[Jan Alonzo]

Created attachment 28132 [details]
DRT - support for dumping the back/forward history items and navigation and back/forward fixes

[Jan Alonzo]

Created attachment 28134 [details]
dump according to the frame's mime type. Also enable the http tests

[Holger Freyther]

Comment on attachment 28134 [details]
dump according to the frame's mime type. Also enable the http tests

> Index: WebKit/WebKit/gtk/webkit/webkitprivate.h
> ===================================================================
> --- WebKit.orig/WebKit/gtk/webkit/webkitprivate.h	2009-03-01 15:56:10.000000000 +1100
> +++ WebKit/WebKit/gtk/webkit/webkitprivate.h	2009-03-01 15:57:53.000000000 +1100
> @@ -158,6 +158,10 @@
>      void
>      webkit_web_policy_decision_cancel (WebKitWebPolicyDecision* decision);
>  
> +    // FIXME: move this functionality into a 'WebKitWebDataSource' once implemented
> +    WEBKIT_API gchar*
> +    webkit_web_frame_get_response_mime_type(WebKitWebFrame* frame);
> +
>      // FIXME: Move these to webkitwebframe.h once their API has been discussed.
>  
>      WEBKIT_API GSList*
> Index: WebKit/WebKit/gtk/webkit/webkitwebframe.cpp
> ===================================================================
> --- WebKit.orig/WebKit/gtk/webkit/webkitwebframe.cpp	2009-03-01 15:58:07.000000000 +1100
> +++ WebKit/WebKit/gtk/webkit/webkitwebframe.cpp	2009-03-01 15:59:16.000000000 +1100
> @@ -5,6 +5,7 @@
>   * Copyright (C) 2008 Christian Dywan <[e-mail]>
>   * Copyright (C) 2008 Collabora Ltd.
>   * Copyright (C) 2008 Nuanti Ltd.
> + * Copyright (C) 2009 Jan Alonzo <[e-mail]>
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Library General Public
> @@ -31,6 +32,7 @@
>  
>  #include "AnimationController.h"
>  #include "CString.h"
> +#include "DocumentLoader.h"
>  #include "FrameLoader.h"
>  #include "FrameLoaderClientGtk.h"
>  #include "FrameTree.h"
> @@ -717,4 +719,12 @@
>      return controller->numberOfActiveAnimations();
>  }
>  
> +gchar* webkit_web_frame_get_response_mime_type(WebKitWebFrame* frame)
> +{
> +    Frame* coreFrame = core(frame);
> +    DocumentLoader* docLoader = coreFrame->loader()->documentLoader();
> +    String mimeType = docLoader->responseMIMEType();
> +    return g_strdup(mimeType.utf8().data());
> +}
> +
>  }
> Index: WebKit/WebKitTools/DumpRenderTree/gtk/DumpRenderTree.cpp
> ===================================================================
> --- WebKit.orig/WebKitTools/DumpRenderTree/gtk/DumpRenderTree.cpp	2009-03-01 16:01:06.000000000 +1100
> +++ WebKit/WebKitTools/DumpRenderTree/gtk/DumpRenderTree.cpp	2009-03-01 16:02:35.000000000 +1100
> @@ -57,6 +57,7 @@
>  extern gchar* webkit_web_frame_get_inner_text(WebKitWebFrame* frame);
>  extern gchar* webkit_web_frame_dump_render_tree(WebKitWebFrame* frame);
>  extern void webkit_web_settings_add_extra_plugin_directory(WebKitWebView* view, const gchar* directory);
> +extern gchar* webkit_web_frame_get_response_mime_type(WebKitWebFrame* frame);
>  }
>  
>  volatile bool done;
> @@ -221,11 +222,13 @@
>  void dump()
>  {
>      invalidateAnyPreviousWaitToDumpWatchdog();

> -        bool dumpAsText = gLayoutTestController->dumpAsText();
> -        // FIXME: Also dump text resuls as text.
> +        dumpAsText = g_ascii_strcasecmp(responseMimeType, "text/plain");

g_free(responseMImeType) missing here?

looks good so far. It might be wise to land this patch last though.

[Holger Freyther]

Comment on attachment 28130 [details]
history item fixes and API additions for dumping a history item


> -    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.

[Holger Freyther]

Comment on attachment 28132 [details]
DRT - support for dumping the back/forward history items and navigation and back/forward fixes


> +static WebKitWebHistoryItem* prevTestBFItem = NULL; // current b/f item at the end of the previous test

please move the comment to the above line.



> +static void dumpHistoryItem(WebKitWebHistoryItem* item, int indent, bool current)
> +{
> +    assert(item != NULL);

ASSERT(item); to use WTF




> +static void dumpBackForwardListForWebView(WebKitWebView* view)
> +{
> +    printf("\n============== Back Forward List ==============\n");
> +    WebKitWebBackForwardList* bfList = webkit_web_view_get_back_forward_list(view);
> +
> +    // Print out all items in the list after prevTestBFItem, which was from the previous test
> +    // Gather items from the end of the list, the print them out from oldest to newest
> +    GList* itemsToPrint = NULL;
> +    gint forwardListCount = webkit_web_back_forward_list_get_forward_length(bfList);
> +    for (int i = forwardListCount; i > 0; i--) {
> +        WebKitWebHistoryItem* item = webkit_web_back_forward_list_get_nth_item(bfList, i);
> +        // something is wrong if the item from the last test is in the forward part of the b/f list
> +        assert(item != prevTestBFItem);

ASSERT?



please coordinate with kov/xan for the release.

[Jan Alonzo]

(In reply to comment #5)
> (From update of attachment 28134 [details] [review])
> > Index: WebKit/WebKit/gtk/webkit/webkitprivate.h
> > ===================================================================
> > --- WebKit.orig/WebKit/gtk/webkit/webkitprivate.h	2009-03-01 15:56:10.000000000 +1100
> > +++ WebKit/WebKit/gtk/webkit/webkitprivate.h	2009-03-01 15:57:53.000000000 +1100
> > @@ -158,6 +158,10 @@
> >      void
> >      webkit_web_policy_decision_cancel (WebKitWebPolicyDecision* decision);
> >  
> > +    // FIXME: move this functionality into a 'WebKitWebDataSource' once implemented
> > +    WEBKIT_API gchar*
> > +    webkit_web_frame_get_response_mime_type(WebKitWebFrame* frame);
> > +
> >      // FIXME: Move these to webkitwebframe.h once their API has been discussed.
> >  
> >      WEBKIT_API GSList*
> > Index: WebKit/WebKit/gtk/webkit/webkitwebframe.cpp
> > ===================================================================
> > --- WebKit.orig/WebKit/gtk/webkit/webkitwebframe.cpp	2009-03-01 15:58:07.000000000 +1100
> > +++ WebKit/WebKit/gtk/webkit/webkitwebframe.cpp	2009-03-01 15:59:16.000000000 +1100
> > @@ -5,6 +5,7 @@
> >   * Copyright (C) 2008 Christian Dywan <[e-mail]>
> >   * Copyright (C) 2008 Collabora Ltd.
> >   * Copyright (C) 2008 Nuanti Ltd.
> > + * Copyright (C) 2009 Jan Alonzo <[e-mail]>
> >   *
> >   * This library is free software; you can redistribute it and/or
> >   * modify it under the terms of the GNU Library General Public
> > @@ -31,6 +32,7 @@
> >  
> >  #include "AnimationController.h"
> >  #include "CString.h"
> > +#include "DocumentLoader.h"
> >  #include "FrameLoader.h"
> >  #include "FrameLoaderClientGtk.h"
> >  #include "FrameTree.h"
> > @@ -717,4 +719,12 @@
> >      return controller->numberOfActiveAnimations();
> >  }
> >  
> > +gchar* webkit_web_frame_get_response_mime_type(WebKitWebFrame* frame)
> > +{
> > +    Frame* coreFrame = core(frame);
> > +    DocumentLoader* docLoader = coreFrame->loader()->documentLoader();
> > +    String mimeType = docLoader->responseMIMEType();
> > +    return g_strdup(mimeType.utf8().data());
> > +}
> > +
> >  }
> > Index: WebKit/WebKitTools/DumpRenderTree/gtk/DumpRenderTree.cpp
> > ===================================================================
> > --- WebKit.orig/WebKitTools/DumpRenderTree/gtk/DumpRenderTree.cpp	2009-03-01 16:01:06.000000000 +1100
> > +++ WebKit/WebKitTools/DumpRenderTree/gtk/DumpRenderTree.cpp	2009-03-01 16:02:35.000000000 +1100
> > @@ -57,6 +57,7 @@
> >  extern gchar* webkit_web_frame_get_inner_text(WebKitWebFrame* frame);
> >  extern gchar* webkit_web_frame_dump_render_tree(WebKitWebFrame* frame);
> >  extern void webkit_web_settings_add_extra_plugin_directory(WebKitWebView* view, const gchar* directory);
> > +extern gchar* webkit_web_frame_get_response_mime_type(WebKitWebFrame* frame);
> >  }
> >  
> >  volatile bool done;
> > @@ -221,11 +222,13 @@
> >  void dump()
> >  {
> >      invalidateAnyPreviousWaitToDumpWatchdog();
> 
> > -        bool dumpAsText = gLayoutTestController->dumpAsText();
> > -        // FIXME: Also dump text resuls as text.
> > +        dumpAsText = g_ascii_strcasecmp(responseMimeType, "text/plain");
> 
> g_free(responseMImeType) missing here?
> 
> looks good so far. It might be wise to land this patch last though.
> 

Landed in r41341

[Jan Alonzo]

(In reply to comment #7)
> (From update of attachment 28132 [details] [review])
> 
> > +static WebKitWebHistoryItem* prevTestBFItem = NULL; // current b/f item at the end of the previous test
> 
> please move the comment to the above line.
> 
> 
> 
> > +static void dumpHistoryItem(WebKitWebHistoryItem* item, int indent, bool current)
> > +{
> > +    assert(item != NULL);
> 
> ASSERT(item); to use WTF
> 
> 
> 
> 
> > +static void dumpBackForwardListForWebView(WebKitWebView* view)
> > +{
> > +    printf("\n============== Back Forward List ==============\n");
> > +    WebKitWebBackForwardList* bfList = webkit_web_view_get_back_forward_list(view);
> > +
> > +    // Print out all items in the list after prevTestBFItem, which was from the previous test
> > +    // Gather items from the end of the list, the print them out from oldest to newest
> > +    GList* itemsToPrint = NULL;
> > +    gint forwardListCount = webkit_web_back_forward_list_get_forward_length(bfList);
> > +    for (int i = forwardListCount; i > 0; i--) {
> > +        WebKitWebHistoryItem* item = webkit_web_back_forward_list_get_nth_item(bfList, i);
> > +        // something is wrong if the item from the last test is in the forward part of the b/f list
> > +        assert(item != prevTestBFItem);
> 
> ASSERT?
> 
> 
> 
> please coordinate with kov/xan for the release.
> 

Landed in r41340

[Jan Alonzo]

(In reply to comment #6)
> (From update of attachment 28130 [details] [review])
> 
> > -    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.
> 

Landed in r41339.

Thanks!