[GTK] Add WebKitWebViewGroup to WebKit2 GTK+ API

• Component: WebKit2
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

In the current API the settings are always shared by all web views, because they are always created on the default page group. This makes impossible to have different settings on different web views. We need to expose the page group in the API, so that users can create web views on a group different than the default one to have their own settings.

[Carlos Garcia Campos]

Created attachment 191131 [details]
Patch

Note that this patch is compatible with current API because web views are created in the default page group by default, so apps currently using the API won't be affected.

[Carlos Garcia Campos]

Adding WebKit2 owners to the CC.

[WebKit Review Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[WebKit Review Bot]

Attachment 191131 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/GNUmakefile.list.am', u'Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitSettingsPrivate.h', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebContextPrivate.h', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebViewGroup.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebViewGroup.h', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebViewGroupPrivate.h', u'Source/WebKit2/UIProcess/API/gtk/docs/webkit2gtk-docs.sgml', u'Source/WebKit2/UIProcess/API/gtk/docs/webkit2gtk-sections.txt', u'Source/WebKit2/UIProcess/API/gtk/docs/webkit2gtk.types', u'Source/WebKit2/UIProcess/API/gtk/tests/GNUmakefile.am', u'Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebView.cpp', u'Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebViewGroup.cpp', u'Source/WebKit2/UIProcess/API/gtk/webkit2.h']" exit_code: 1
WARNING: File exempt from style guide. Skipping: "Source/WebKit2/UIProcess/API/gtk/WebKitWebViewGroup.h"
WARNING: File exempt from style guide. Skipping: "Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h"
Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebViewGroup.cpp:74:  Use 0 instead of NULL.  [readability/null] [5]
WARNING: File exempt from style guide. Skipping: "Source/WebKit2/UIProcess/API/gtk/webkit2.h"
Total errors found: 1 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Carlos Garcia Campos]

(In reply to comment #4)
> Attachment 191131 [details] did not pass style-queue:
> 
> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebViewGroup.cpp:74:  Use 0 instead of NULL.  [readability/null] [5]

This NULL is a sentinel, we can't use 0 ins this particular case.

[Carlos Garcia Campos]

It seems to me that GTK+ EWS is failing because it builds with WebKit2 disabled, but tries to generate the API docs for WebKit2. 

Failed to run "['Tools/Scripts/build-webkit', '--release', '--gtk', '--update-gtk', '--no-webkit2', '--makeargs="-j8"']" exit_code: 1

Copying template files to output directory...
Running gtkdoc-scan
Running gtkdoc-scangobj
webkit2gtk-scan.o: In function `get_object_types':
webkit2gtk-scan.c:(.text+0x307): undefined reference to `webkit_web_view_group_get_type'
collect2: error: ld returned 1 exit status
Linking of scanner failed: 

I ran generate-gtkdoc locally and it worked.

[Carlos Garcia Campos]

Now the box is green, I don't understand what's going on with GTK EWS

[Martin Robinson]

Comment on attachment 191131 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=191131&action=review

Very nice. Looks good to me.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:624
> +            _("Web View Group"),

Nit: WebView

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:625
> +            _("The web view group of the view"),

Maybe this should say "The WebKitWebViewGroup of the view?"

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewGroup.cpp:41
> + * To create a #WebKitWebView on a different WebKitWebViewGroup you can use

Nit: on -> in

[Gustavo Noronha (kov)]

I think the GTK+ EWS that failed to build (rego's) did not finish reporting the error for some reason, since there is no comment in the bug and the patch did not get cq-, then another EWS bot got the patch from the queue and it passed.

[Gustavo Noronha (kov)]

Comment on attachment 191131 [details]
Patch

WebCore makes a distinction of per-Page settings and PageGroup settings, so that you can still have different settings that make sense for each page, but can also have PageGroup-wide settings (such as local storage path), have you considered using that same approach at the API level?

[Carlos Garcia Campos]

(In reply to comment #10)
> (From update of attachment 191131 [details])
> WebCore makes a distinction of per-Page settings and PageGroup settings, so that you can still have different settings that make sense for each page, but can also have PageGroup-wide settings (such as local storage path), have you considered using that same approach at the API level?

There are no GroupSettings in WebKit2, I think those are per process and handled by the WebContext.

[Manuel Rego Casasnovas]

(In reply to comment #9)
> I think the GTK+ EWS that failed to build (rego's) did not finish reporting the error for some reason, since there is no comment in the bug and the patch did not get cq-, then another EWS bot got the patch from the queue and it passed.

My EWS doesn't have EditBugs permission yet, that's why even if it fails it doesn't add comments to the bug. I've requested it several times by IRC without success :-(. I'll try again.

[Gustavo Noronha (kov)]

(In reply to comment #12)
> My EWS doesn't have EditBugs permission yet, that's why even if it fails it doesn't add comments to the bug. I've requested it several times by IRC without success :-(. I'll try again.

Would be good to try to figure out why it failed, though, while the other EWS passed, perhaps it has left-overs that caused webkit2 docs to be built for some reason?

[Manuel Rego Casasnovas]

(In reply to comment #13)
> (In reply to comment #12)
> > My EWS doesn't have EditBugs permission yet, that's why even if it fails it doesn't add comments to the bug. I've requested it several times by IRC without success :-(. I'll try again.
> 
> Would be good to try to figure out why it failed, though, while the other EWS passed, perhaps it has left-overs that caused webkit2 docs to be built for some reason?

I've tried again the patch in the EWS and it failed again:
Generating WebKit2 documentation...
Copying template files to output directory...
Running gtkdoc-scan
Running gtkdoc-scangobj
webkit2gtk-scan.o: In function `get_object_types':
webkit2gtk-scan.c:(.text+0x307): undefined reference to `webkit_web_view_group_get_type'
collect2: error: ld returned 1 exit status
Linking of scanner failed: 
Traceback (most recent call last):
  File "/home/igalia/mrego/WebKit/Tools/gtk/generate-gtkdoc", line 203, in <module>
    saw_webkit2_warnings = generate_doc(generator)
  File "/home/igalia/mrego/WebKit/Tools/gtk/generate-gtkdoc", line 154, in generate_doc
    generator.generate(html='--skip-html' not in sys.argv)
  File "/home/igalia/mrego/WebKit/Tools/gtk/gtkdoc.py", line 137, in generate
    self._run_gtkdoc_scangobj()
  File "/home/igalia/mrego/WebKit/Tools/gtk/gtkdoc.py", line 314, in _run_gtkdoc_scangobj
    env=env, cwd=self.output_dir)
  File "/home/igalia/mrego/WebKit/Tools/gtk/gtkdoc.py", line 198, in _run_command
    % (args[0], process.returncode))
Exception: gtkdoc-scangobj produced a non-zero return code 1

 gtkdoc did not build without warnings


The real problem was that I maybe did a build without "--disable-webkit2" in the EWS before, and that was the reason why the file "WebKitBuild/Release/Source/WebKit2/webkit2gtk-3.0.pc" exists and it tried to generate WK2 documentation.

I did a clean build which fixed the problem. So, sorry for the noise.

[Martin Robinson]

Other than the few nits, I think this is ready for owner review. Anders or Sam, do you mind double-checking that this is okay with you?

[Carlos Garcia Campos]

Created attachment 199241 [details]
Rebased patch

Rebased and addressed review nits. Ping WebKit2 owners

[WebKit Commit Bot]

Attachment 199241 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/GNUmakefile.list.am', u'Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitSettingsPrivate.h', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebContextPrivate.h', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebViewGroup.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebViewGroup.h', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebViewGroupPrivate.h', u'Source/WebKit2/UIProcess/API/gtk/docs/webkit2gtk-docs.sgml', u'Source/WebKit2/UIProcess/API/gtk/docs/webkit2gtk-sections.txt', u'Source/WebKit2/UIProcess/API/gtk/docs/webkit2gtk.types', u'Source/WebKit2/UIProcess/API/gtk/tests/GNUmakefile.am', u'Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebView.cpp', u'Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebViewGroup.cpp', u'Source/WebKit2/UIProcess/API/gtk/webkit2.h']" exit_code: 1
WARNING: File exempt from style guide. Skipping: "Source/WebKit2/UIProcess/API/gtk/WebKitWebViewGroup.h"
WARNING: File exempt from style guide. Skipping: "Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h"
Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebViewGroup.cpp:74:  Use 0 instead of NULL.  [readability/null] [5]
WARNING: File exempt from style guide. Skipping: "Source/WebKit2/UIProcess/API/gtk/webkit2.h"
Total errors found: 1 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Carlos Garcia Campos]

gtk-wk2 failure seems to be related to freedesktop.org git issues while updating dependencies.

[Anders Carlsson]

Comment on attachment 199241 [details]
Rebased patch

View in context: https://bugs.webkit.org/attachment.cgi?id=199241&action=review

> Source/WebKit2/ChangeLog:75
> +        (webkit_web_view_group_get_settings): Get the settins of a

Typo, settins.

[Carlos Garcia Campos]

Committed r149117: <http://trac.webkit.org/changeset/149117>