Bug 24259 - [Gtk] get the HTTP layout tests going
Summary: [Gtk] get the HTTP layout tests going
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Jan Alonzo
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2009-02-28 04:03 PST by Jan Alonzo
Modified: 2009-03-01 11:25 PST (History)
0 users

See Also:


Attachments
added necessary API additions and fixes for dumping the back/forward list, etc.. (25.43 KB, patch)
2009-02-28 04:21 PST, Jan Alonzo
no flags Details | Formatted Diff | Diff
history item fixes and API additions for dumping a history item (9.31 KB, patch)
2009-02-28 20:24 PST, Jan Alonzo
zecke: review+
Details | Formatted Diff | Diff
DRT - support for dumping the back/forward history items and navigation and back/forward fixes (7.62 KB, patch)
2009-02-28 20:51 PST, Jan Alonzo
zecke: review+
Details | Formatted Diff | Diff
dump according to the frame's mime type. Also enable the http tests (8.61 KB, patch)
2009-02-28 21:21 PST, Jan Alonzo
zecke: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Alonzo 2009-02-28 04:03:23 PST
We should get the HTTP tests working so we can test navigation, xhr, etc...
Comment 1 Jan Alonzo 2009-02-28 04:21:31 PST
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.
Comment 2 Jan Alonzo 2009-02-28 20:24:45 PST
Created attachment 28130 [details]
history item fixes and API additions for dumping a history item
Comment 3 Jan Alonzo 2009-02-28 20:51:14 PST
Created attachment 28132 [details]
DRT - support for dumping the back/forward history items and navigation and back/forward fixes
Comment 4 Jan Alonzo 2009-02-28 21:21:20 PST
Created attachment 28134 [details]
dump according to the frame's mime type. Also enable the http tests
Comment 5 Holger Freyther 2009-03-01 07:13:06 PST
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 <christian@imendio.com>
>   * Copyright (C) 2008 Collabora Ltd.
>   * Copyright (C) 2008 Nuanti Ltd.
> + * Copyright (C) 2009 Jan Alonzo <jmalonzo@gmail.com>
>   *
>   * 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.
Comment 6 Holger Freyther 2009-03-01 08:07:07 PST
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.
Comment 7 Holger Freyther 2009-03-01 08:26:54 PST
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.
Comment 8 Jan Alonzo 2009-03-01 11:23:15 PST
(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 <christian@imendio.com>
> >   * Copyright (C) 2008 Collabora Ltd.
> >   * Copyright (C) 2008 Nuanti Ltd.
> > + * Copyright (C) 2009 Jan Alonzo <jmalonzo@gmail.com>
> >   *
> >   * 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
Comment 9 Jan Alonzo 2009-03-01 11:24:12 PST
(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
Comment 10 Jan Alonzo 2009-03-01 11:25:03 PST
(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!