Bug 151418 – Remove the non-NetworkProcess configurations

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Alex Christensen]

NetworkProcess should be the default.  Supporting non-NetworkProcess code paths makes the code messy.

[Alex Christensen]

Created attachment 265889 [details]
Patch

[Anders Carlsson]

Comment on attachment 265889 [details]
Patch

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

> Source/WebKit2/UIProcess/API/C/WKContext.cpp:-340
> -void WKContextSetProcessModel(WKContextRef contextRef, WKProcessModel processModel)
> -{
> -    toImpl(contextRef)->setProcessModel(toProcessModel(processModel));
> -}
> -
> -WKProcessModel WKContextGetProcessModel(WKContextRef contextRef)
> -{
> -    return toAPI(toImpl(contextRef)->processModel());
> -}

You can't remove this, it'll break nightlies. Just make these no-ops and move them to WKDeprecatedFunctions.cpp.

> Source/WebKit2/UIProcess/API/C/WKContext.cpp:-560
> -void WKContextSetUsesNetworkProcess(WKContextRef contextRef, bool usesNetworkProcess)
> -{
> -    toImpl(contextRef)->setUsesNetworkProcess(usesNetworkProcess);
> -}

Ditto.

> Source/WebKit2/UIProcess/API/C/WKContext.h:81
>  enum {
> -    kWKProcessModelSharedSecondaryProcess = 0,
> -    kWKProcessModelMultipleSecondaryProcesses = 1
> -};
> -typedef uint32_t WKProcessModel;
> -
> -enum {
>      kWKStatisticsOptionsWebContent = 1 << 0,
>      kWKStatisticsOptionsNetworking = 1 << 1
>  };

Pretty sure removing this is going to break the Mavericks build of Safari.

> Source/WebKit2/UIProcess/API/C/WKContextPrivate.h:-80
> -// FIXME: This function is temporary and useful during the development of the NetworkProcess feature.
> -// At some point it should be removed.
> -WK_EXPORT void WKContextSetUsesNetworkProcess(WKContextRef context, bool usesNetworkProcess);

Ditto.

[Geoffrey Garen]

Comment on attachment 265889 [details]
Patch

r=me if you make it build

Last 500 characters of output:
it2/UIProcess/API/efl/ewk_context.cpp:284:48: error: 'toEwkProcessModel' declared as an 'inline' variable
 inline Ewk_Process_Model toEwkProcessModel(WKProcessModel processModel)
                                                ^
../../Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:284:48: error: 'WKProcessModel' was not declared in this scope
../../Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:285:1: error: expected ',' or ';' before '{' token
 {
 ^
ninja: build stopped: subcommand failed.

Failed to run "['Tools/Scripts/build-webkit', '--release', '--efl', '--update-efl', '--makeargs="-j8"']" exit_code: 1
/Root/include/atk-1.0 -isystem ../DependenciesEFL/Root/include/eldbus-1 -isystem /usr/include/dbus-1.0 -isystem /usr/lib/x86_64-linux-gnu/dbus-1.0/include    -Werror -Wall -Wextra -Wcast-align -Wformat-security -Wmissing-format-attribute -Wpointer-arith -Wundef -Wwrite-strings  -Wno-unused-parameter -MMD -MT Source/WebKit2/CMakeFiles/WebKit2.dir/UIProcess/API/efl/ewk_context.cpp.o -MF Source/WebKit2/CMakeFiles/WebKit2.dir/UIProcess/API/efl/ewk_context.cpp.o.d -o Source/WebKit2/CMakeFiles/WebKit2.dir/UIProcess/API/efl/ewk_context.cpp.o -c ../../Source/WebKit2/UIProcess/API/efl/ewk_context.cpp
../../Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:260:12: error: 'WKProcessModel' does not name a type
 inline WKProcessModel toWKProcessModel(Ewk_Process_Model processModel)
            ^
../../Source/WebKit2/UIProcess/API/efl/ewk_context.cpp: In member function 'void EwkContext::setProcessModel(Ewk_Process_Model)':
../../Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:275:5: error: 'WKProcessModel' was not declared in this scope
     WKProcessModel newWKProcessModel = toWKProcessModel(processModel);
     ^

[Alex Christensen]

Created attachment 265900 [details]
Patch

[Alex Christensen]

Created attachment 265904 [details]
Patch

[Alex Christensen]

Created attachment 265909 [details]
Patch

[Alex Christensen]

Created attachment 265912 [details]
Patch

[Alex Christensen]

Created attachment 265913 [details]
Patch

[Alex Christensen]

Created attachment 265914 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 265914 [details]
Patch

Rejecting attachment 265914 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 265914, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Geoff Garen found in /Volumes/Data/EWS/WebKit/ChangeLog does not appear to be a valid reviewer according to contributors.json.
/Volumes/Data/EWS/WebKit/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://webkit-queues.webkit.org/results/451825

[Alex Christensen]

Created attachment 265915 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 265915 [details]
Patch

Clearing flags on attachment: 265915

Committed r192667: <http://trac.webkit.org/changeset/192667>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alex Christensen]

GTK build fixed in http://trac.webkit.org/changeset/192668
I'm looking into the API test failures now...

[Michael Catanzaro]

(In reply to comment #14)
> GTK build fixed in http://trac.webkit.org/changeset/192668
> I'm looking into the API test failures now...

I haven't looked, but we probably have a test that makes sure webkit_web_context_get_process_model() after webkit_web_context_set_process_model() returns the process model that was set. Please don't worry about any GTK+ API tests you might have broken; we'll fix them.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 151476

[Carlos Garcia Campos]

(In reply to comment #15)
> (In reply to comment #14)
> > GTK build fixed in http://trac.webkit.org/changeset/192668
> > I'm looking into the API test failures now...
> 
> I haven't looked, but we probably have a test that makes sure
> webkit_web_context_get_process_model() after
> webkit_web_context_set_process_model() returns the process model that was
> set. Please don't worry about any GTK+ API tests you might have broken;
> we'll fix them.

A unit test failure probably means we would be changing the behaviour of our public API, we should ensure this change is backwards compatible. See my review on bug #151473.

[Csaba Osztrogonác]

(In reply to comment #12)
> Comment on attachment 265915 [details]
> Patch
> 
> Clearing flags on attachment: 265915
> 
> Committed r192667: <http://trac.webkit.org/changeset/192667>

It made the EFL bot early exit due to 50+ timeouts:
https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2/builds/25632

It would be great if the maintainers could fix this issue before relanding 
this patch. Maybe EFL needs some refactoring similar to GTK port.

[Carlos Garcia Campos]

Created attachment 265948 [details]
Patch with the GTK changes needed

This doesn't fix the unit tests timing out, but those tests don't work with using the network process for some reason. We can deal with them in a different bug.

[Carlos Garcia Campos]

(In reply to comment #19)
> Created attachment 265948 [details]
> Patch with the GTK changes needed
> 
> This doesn't fix the unit tests timing out, but those tests don't work with
> using the network process for some reason. We can deal with them in a
> different bug.

https://bugs.webkit.org/show_bug.cgi?id=151490

[Alex Christensen]

I'm going to do this in pieces, starting with https://bugs.webkit.org/show_bug.cgi?id=151512 which shouldn't change the compiled output at all.

[Carlos Garcia Campos]

(In reply to comment #21)
> I'm going to do this in pieces, starting with
> https://bugs.webkit.org/show_bug.cgi?id=151512 which shouldn't change the
> compiled output at all.

Ok, I've moved the GTK+ changes to its own bug then. It switches to use network process unconditionally in preparation for this change, so that when you fix this bug you only need to remove a few lines in the GTK+ port to make it build.

https://bugs.webkit.org/show_bug.cgi?id=151541

[Alex Christensen]

(In reply to comment #22)
Thanks Carlos!

[Alex Christensen]

This is finished because all the dependent bugs are finished.  I've completely landed this in pieces.