Bug 151418 - Remove the non-NetworkProcess configurations
Summary: Remove the non-NetworkProcess configurations
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords:
Depends on: 151476 151512 151541 151580 151662
Blocks: 150834 140075 146552 151473
  Show dependency treegraph
 
Reported: 2015-11-18 15:57 PST by Alex Christensen
Modified: 2015-11-30 15:37 PST (History)
8 users (show)

See Also:


Attachments
Patch (146.89 KB, patch)
2015-11-19 13:00 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (146.70 KB, patch)
2015-11-19 14:05 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (145.61 KB, patch)
2015-11-19 14:29 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (164.95 KB, patch)
2015-11-19 15:20 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (165.31 KB, patch)
2015-11-19 15:31 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (165.68 KB, patch)
2015-11-19 15:38 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (165.60 KB, patch)
2015-11-19 15:50 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (165.71 KB, patch)
2015-11-19 16:01 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch with the GTK changes needed (173.41 KB, patch)
2015-11-20 04:51 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2015-11-18 15:57:18 PST
NetworkProcess should be the default.  Supporting non-NetworkProcess code paths makes the code messy.
Comment 1 Alex Christensen 2015-11-19 13:00:31 PST
Created attachment 265889 [details]
Patch
Comment 2 Anders Carlsson 2015-11-19 13:06:03 PST
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.
Comment 3 Geoffrey Garen 2015-11-19 13:49:31 PST
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);
     ^
Comment 4 Alex Christensen 2015-11-19 14:05:16 PST
Created attachment 265900 [details]
Patch
Comment 5 Alex Christensen 2015-11-19 14:29:41 PST
Created attachment 265904 [details]
Patch
Comment 6 Alex Christensen 2015-11-19 15:20:59 PST
Created attachment 265909 [details]
Patch
Comment 7 Alex Christensen 2015-11-19 15:31:05 PST
Created attachment 265912 [details]
Patch
Comment 8 Alex Christensen 2015-11-19 15:38:41 PST
Created attachment 265913 [details]
Patch
Comment 9 Alex Christensen 2015-11-19 15:50:51 PST
Created attachment 265914 [details]
Patch
Comment 10 WebKit Commit Bot 2015-11-19 15:57:04 PST
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
Comment 11 Alex Christensen 2015-11-19 16:01:16 PST
Created attachment 265915 [details]
Patch
Comment 12 WebKit Commit Bot 2015-11-19 16:51:49 PST
Comment on attachment 265915 [details]
Patch

Clearing flags on attachment: 265915

Committed r192667: <http://trac.webkit.org/changeset/192667>
Comment 13 WebKit Commit Bot 2015-11-19 16:51:53 PST
All reviewed patches have been landed.  Closing bug.
Comment 14 Alex Christensen 2015-11-19 17:28:11 PST
GTK build fixed in http://trac.webkit.org/changeset/192668
I'm looking into the API test failures now...
Comment 15 Michael Catanzaro 2015-11-19 18:50:20 PST
(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.
Comment 16 WebKit Commit Bot 2015-11-19 19:49:15 PST
Re-opened since this is blocked by bug 151476
Comment 17 Carlos Garcia Campos 2015-11-19 23:29:28 PST
(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.
Comment 18 Csaba Osztrogonác 2015-11-20 02:49:07 PST
(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.
Comment 19 Carlos Garcia Campos 2015-11-20 04:51:01 PST
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.
Comment 20 Carlos Garcia Campos 2015-11-20 05:00:52 PST
(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
Comment 21 Alex Christensen 2015-11-20 13:59:55 PST
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.
Comment 22 Carlos Garcia Campos 2015-11-22 02:26:37 PST
(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
Comment 23 Alex Christensen 2015-11-23 21:41:27 PST
(In reply to comment #22)
Thanks Carlos!
Comment 24 Alex Christensen 2015-11-30 15:37:00 PST
This is finished because all the dependent bugs are finished.  I've completely landed this in pieces.