Bug 57820 - [GTK] Export an API similar to WebKit1
: [GTK] Export an API similar to WebKit1
Status: RESOLVED FIXED
: WebKit
WebKit2
: 528+ (Nightly build)
: PC Linux
: P2 Normal
Assigned To:
:
:
: 57944
: 57835
  Show dependency treegraph
 
Reported: 2011-04-05 01:41 PST by
Modified: 2011-06-13 01:57 PST (History)


Attachments
Patch (107.24 KB, patch)
2011-04-05 01:50 PST, Carlos Garcia Campos
no flags Review Patch | Details | Formatted Diff | Diff
Patch to add WebKitWebView (15.07 KB, patch)
2011-04-06 06:27 PST, Carlos Garcia Campos
no flags Review Patch | Details | Formatted Diff | Diff
Updated patch (14.50 KB, patch)
2011-04-15 02:42 PST, Carlos Garcia Campos
no flags Review Patch | Details | Formatted Diff | Diff
Updated patch (16.55 KB, patch)
2011-04-25 02:36 PST, Carlos Garcia Campos
no flags Review Patch | Details | Formatted Diff | Diff
New patch updated to current git master (18.96 KB, patch)
2011-04-29 05:32 PST, Carlos Garcia Campos
no flags Review Patch | Details | Formatted Diff | Diff
Updated patch to current git master (17.21 KB, patch)
2011-05-03 04:05 PST, Carlos Garcia Campos
no flags Review Patch | Details | Formatted Diff | Diff
Patch rebased to current git master (22.29 KB, patch)
2011-06-10 03:50 PST, Carlos Garcia Campos
mrobinson: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-04-05 01:41:48 PST
Currently the minibrowser is using the common C API, we should build an API, similar to WebKit1 API or the same if possible, on top of the common C API.
------- Comment #1 From 2011-04-05 01:50:42 PST -------
Created an attachment (id=88190) [details]
Patch

Move gtk API from UIProcess/gtk to UIProcess/API/gtk. WebView and WebViewWidget have been merged into WebKitWebView, the public view widget that provides the same API than WebKit1. PageClient implementation has been moved to PageClientImpl. WKView and WKAPICastGtk have been removed since they are no longer needed.
------- Comment #2 From 2011-04-05 01:51:47 PST -------
The patch applies on top of patch attached to bug #54230
------- Comment #3 From 2011-04-05 08:24:41 PST -------
(From update of attachment 88190 [details])
We want to support both the WebKit2-style and WebKit1-style API. Thus, the WebKit1 API should sit fully on top of the WebKit2 API. The WebKit1-style widget needs it's own Clients, so I'd like to keep it separate from the WebKit2-widget, which should be mostly private.
------- Comment #4 From 2011-04-06 06:27:21 PST -------
Created an attachment (id=88406) [details]
Patch to add WebKitWebView

This patch adds WebKitWebView on top of WebKitWebViewBase (see patch attached to bug #57944). It also enables GtkLauncher build for webkit2. There are some runtime warnings when running on webkit2, but they are harmless.
------- Comment #5 From 2011-04-15 02:42:21 PST -------
Created an attachment (id=89759) [details]
Updated patch

Patch updated to current git master.
------- Comment #6 From 2011-04-25 02:36:11 PST -------
Created an attachment (id=90909) [details]
Updated patch

New patch, the only difference with previous one is that this one includes a webkit.h file for compatibility with webkit1 gtk API. GtkLauncher still uses a macro, but only for missing features, so it will be removed soon and the code will be exactly the same for webkit1 and webkit2.
------- Comment #7 From 2011-04-26 16:16:11 PST -------
(From update of attachment 90909 [details])
Going to remove the review flag from this one, since the changes to the dependency will almost certainly mean this needs a new patch.
------- Comment #8 From 2011-04-29 05:32:45 PST -------
Created an attachment (id=91669) [details]
New patch updated to current git master

Instead of using the same GtkLauncher, it builds GtkLauncher2 so that when bug #59695 is fixed there will be two different binaries.
------- Comment #9 From 2011-04-29 05:35:34 PST -------
Attachment 91669 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'GNUmakefile.am', u'Source/We..." exit_code: 1

Source/WebKit2/UIProcess/API/gtk/webkit/webkit.h:26:  #ifndef header guard has wrong style, please use: WTF_webkit_h  [build/header_guard] [5]
Source/WebKit2/UIProcess/API/gtk/webkit/webkit.h:31:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:55:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:56:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:57:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:58:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:59:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:60:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:61:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:62:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:72:  The parameter name "webView" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:75:  The parameter name "webView" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:78:  The parameter name "webView" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:40:  webkit_web_view_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:45:  webkit_web_view_class_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:55:  webkit_web_view_load_uri is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:64:  webkit_web_view_go_back is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:72:  webkit_web_view_go_forward is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 18 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
------- Comment #10 From 2011-05-02 13:12:27 PST -------
(From update of attachment 91669 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=91669&action=review

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:16
> +/*
> + * Copyright (C) 2011 Igalia S.L.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
> + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
> + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS

Instead of duplicating this header, would it be possible to reuse the one from Webkit1?
------- Comment #11 From 2011-05-02 23:52:49 PST -------
(In reply to comment #10)
> (From update of attachment 91669 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=91669&action=review
> 
> > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:16
> > +/*
> > + * Copyright (C) 2011 Igalia S.L.
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions
> > + * are met:
> > + * 1. Redistributions of source code must retain the above copyright
> > + *    notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above copyright
> > + *    notice, this list of conditions and the following disclaimer in the
> > + *    documentation and/or other materials provided with the distribution.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
> > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
> > + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
> > + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
> 
> Instead of duplicating this header, would it be possible to reuse the one from Webkit1?

Yes, sure.
------- Comment #12 From 2011-05-03 04:05:17 PST -------
Created an attachment (id=92059) [details]
Updated patch to current git master
------- Comment #13 From 2011-05-03 10:45:43 PST -------
(From update of attachment 92059 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=92059&action=review

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:21
> +#include "WebKitWebView.h"

Why not just put the WebKit1 header in the include path instead of completely duplicating the header?
------- Comment #14 From 2011-05-03 11:12:06 PST -------
Attachment 92059 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:52:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:53:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:54:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:55:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:56:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:57:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:58:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:59:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:69:  The parameter name "webView" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:72:  The parameter name "webView" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:75:  The parameter name "webView" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit2/UIProcess/API/gtk/webkit/webkit.h:20:  #ifndef header guard has wrong style, please use: WTF_webkit_h  [build/header_guard] [5]
Source/WebKit2/UIProcess/API/gtk/webkit/webkit.h:25:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:34:  webkit_web_view_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:39:  webkit_web_view_class_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:49:  webkit_web_view_load_uri is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:58:  webkit_web_view_go_back is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:66:  webkit_web_view_go_forward is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 18 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
------- Comment #15 From 2011-05-03 23:38:35 PST -------
(In reply to comment #13)
> (From update of attachment 92059 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=92059&action=review
> 
> > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:21
> > +#include "WebKitWebView.h"
> 
> Why not just put the WebKit1 header in the include path instead of completely duplicating the header?

Because headers might be different in the future. The existing API in webkit1 will be kept in webkit2 to ensure the compatibility, but we might want to add new methods to the API that don't make sense in webkit1. For example, when we add a wrapper for WKContext, we could add webkit_web_view_new_with_context() to the webkit2 API.
------- Comment #16 From 2011-05-11 17:00:55 PST -------
(In reply to comment #15)
> (In reply to comment #13)
> > (From update of attachment 92059 [details] [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=92059&action=review
> > 
> > > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:21
> > > +#include "WebKitWebView.h"
> > 
> > Why not just put the WebKit1 header in the include path instead of completely duplicating the header?
> 
> Because headers might be different in the future. The existing API in webkit1 will be kept in webkit2 to ensure the compatibility, but we might want to add new methods to the API that don't make sense in webkit1. For example, when we add a wrapper for WKContext, we could add webkit_web_view_new_with_context() to the webkit2 API.

For now, I think it would be better to explore sharing as much of the headers as possible. If we need to deviate from the WebKit1 header we could simply have a wrapper include.
------- Comment #17 From 2011-05-11 23:53:01 PST -------
(In reply to comment #16)
> (In reply to comment #15)
> > (In reply to comment #13)
> > > (From update of attachment 92059 [details] [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=92059&action=review
> > > 
> > > > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:21
> > > > +#include "WebKitWebView.h"
> > > 
> > > Why not just put the WebKit1 header in the include path instead of completely duplicating the header?
> > 
> > Because headers might be different in the future. The existing API in webkit1 will be kept in webkit2 to ensure the compatibility, but we might want to add new methods to the API that don't make sense in webkit1. For example, when we add a wrapper for WKContext, we could add webkit_web_view_new_with_context() to the webkit2 API.
> 
> For now, I think it would be better to explore sharing as much of the headers as possible. If we need to deviate from the WebKit1 header we could simply have a wrapper include.

How can we share a header if they are not exactly the same? both apis are supposed to be parallel installable even if the apis are similar. If we add an #ifdef WEBKIT2 to all headers, clients will have to pass that macro somehow when building.
------- Comment #18 From 2011-06-10 01:30:22 PST -------
(In reply to comment #16)
> 
> For now, I think it would be better to explore sharing as much of the headers as possible. If we need to deviate from the WebKit1 header we could simply have a wrapper include.

I agree sharing as much code as possible makes sense, but I think having exactly the same header for both is not possible even to begin with (among others for the reasons Carlos has explained). So IMHO some form of shared header that both wk1 and wk2 include could be the way to go.
------- Comment #19 From 2011-06-10 03:50:43 PST -------
Created an attachment (id=96727) [details]
Patch rebased to current git master

As suggested by xan, I've moved the common declarations to a common header file that is part of webkit1, but included from webkit2 api.
------- Comment #20 From 2011-06-10 03:54:04 PST -------
Attachment 96727 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/gtk/ChangeLog', u'Source/Web..." exit_code: 1

Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:52:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:53:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:54:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:55:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:56:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:57:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:58:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:59:  Extra space before ( in function call  [whitespace/parens] [4]
Source/WebKit2/UIProcess/API/gtk/webkit/webkit.h:20:  #ifndef header guard has wrong style, please use: WTF_webkit_h  [build/header_guard] [5]
Source/WebKit2/UIProcess/API/gtk/webkit/webkit.h:25:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:34:  webkit_web_view_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:39:  webkit_web_view_class_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:49:  webkit_web_view_load_uri is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:58:  webkit_web_view_go_back is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:66:  webkit_web_view_go_forward is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 15 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.
------- Comment #21 From 2011-06-10 07:58:22 PST -------
(From update of attachment 96727 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=96727&action=review

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:44
> +GtkWidget* webkit_web_view_new(void)

I think we should avoid the void for empty parameter lists in C++ code, if possible.
------- Comment #22 From 2011-06-10 07:58:38 PST -------
Look good!
------- Comment #23 From 2011-06-13 01:57:11 PST -------
Committed r88631: <http://trac.webkit.org/changeset/88631>