Bug 194551 - [WPE][GTK] Merge WebProcessPoolWPE.cpp and WebProcessPoolGtk.cpp
Summary: [WPE][GTK] Merge WebProcessPoolWPE.cpp and WebProcessPoolGtk.cpp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: PC Linux
: P2 Normal
Assignee: Michael Catanzaro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-02-12 12:45 PST by Michael Catanzaro
Modified: 2019-02-13 14:49 PST (History)
3 users (show)

See Also:


Attachments
Patch (15.80 KB, patch)
2019-02-12 12:48 PST, Michael Catanzaro
cgarcia: review+
cgarcia: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2019-02-12 12:45:52 PST
Merge WebProcessPoolWPE and WebProcessPoolGtk since the files are almost entirely duplicated.
Comment 1 Michael Catanzaro 2019-02-12 12:48:19 PST
Created attachment 361820 [details]
Patch
Comment 2 Carlos Garcia Campos 2019-02-12 23:30:11 PST
Comment on attachment 361820 [details]
Patch

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

> Source/WebKit/UIProcess/glib/WebProcessPoolGLib.cpp:43
> +#include "APIProcessPoolConfiguration.h"
> +#include "Logging.h"
> +#include "WebCookieManagerProxy.h"
> +#include "WebMemoryPressureHandler.h"
> +#include "WebProcessCreationParameters.h"
> +#include "WebProcessMessages.h"
> +#include <JavaScriptCore/RemoteInspectorServer.h>
> +#include <WebCore/GStreamerCommon.h>
> +#include <WebCore/NotImplemented.h>
> +#include <WebCore/SchemeRegistry.h>
> +#include <wtf/FileSystem.h>
> +#include <wtf/glib/GUniquePtr.h>
> +#include <wtf/text/CString.h>

This is a good opportunity to clean up the include headers, I'm sure we can remove some of them.

> Source/WebKit/UIProcess/glib/WebProcessPoolGLib.cpp:101
> +    // This is misnamed. It can only be used to disable complex text.
> +    parameters.shouldAlwaysUseComplexTextCodePath = true;
> +    const char* forceComplexText = getenv("WEBKIT_FORCE_COMPLEX_TEXT");
> +    if (forceComplexText && !strcmp(forceComplexText, "0"))

It's only for debugging, so we can just rename it. There isn't any script currently using it (WKTR used it to disable complex text in layout tests). I think we can use it in wpe too (in that case to force complex text).
Comment 3 Michael Catanzaro 2019-02-13 14:35:05 PST
(In reply to Carlos Garcia Campos from comment #2)
> This is a good opportunity to clean up the include headers, I'm sure we can
> remove some of them.

You're right, half of them are stale. I got it down to:

#include "WebMemoryPressureHandler.h"
#include "WebProcessCreationParameters.h"
#include <JavaScriptCore/RemoteInspectorServer.h>
#include <WebCore/GStreamerCommon.h>
#include <wtf/glib/GUniquePtr.h>

Though we're probably missing some we ought to have, thanks to unified build.

> > Source/WebKit/UIProcess/glib/WebProcessPoolGLib.cpp:101
> > +    // This is misnamed. It can only be used to disable complex text.
> > +    parameters.shouldAlwaysUseComplexTextCodePath = true;
> > +    const char* forceComplexText = getenv("WEBKIT_FORCE_COMPLEX_TEXT");
> > +    if (forceComplexText && !strcmp(forceComplexText, "0"))
> 
> It's only for debugging, so we can just rename it. There isn't any script
> currently using it (WKTR used it to disable complex text in layout tests). I
> think we can use it in wpe too (in that case to force complex text).

I'll do a follow-up.
Comment 4 Michael Catanzaro 2019-02-13 14:35:20 PST
Committed r241474: <https://trac.webkit.org/changeset/241474>
Comment 5 Michael Catanzaro 2019-02-13 14:49:40 PST
(In reply to Michael Catanzaro from comment #3)
> I'll do a follow-up.

Bug #194614