Bug 179538

Summary:

webkitpy: Separate AsyncProcess from ServerProcess

Product:

WebKit

Reporter:

Jonathan Bedard <jbedard>

Component:

Tools / Tests

Assignee:

Jonathan Bedard <jbedard>

Status:

RESOLVED WONTFIX

Severity:

Normal

CC:

aakash_jain, buildbot, ddkilzer, glenn, lforschler, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	jbedard: review-, jbedard: commit-queue-

Jonathan Bedard

Reported 2017-11-10 10:58:43 PST

Requiring ServerProcess to be passed a port object makes this class difficult to use outside of running layout tests.

Attachments
Patch (18.05 KB, patch) 2017-11-10 11:07 PST, Jonathan Bedard	no flags	Details Formatted Diff Diff
Patch (18.02 KB, patch) 2017-11-10 11:48 PST, Jonathan Bedard	no flags	Details Formatted Diff Diff
Patch (18.02 KB, patch) 2017-11-10 12:26 PST, Jonathan Bedard	no flags	Details Formatted Diff Diff
Patch (45.69 KB, patch) 2017-11-16 17:07 PST, Jonathan Bedard	no flags	Details Formatted Diff Diff
Patch (48.72 KB, patch) 2017-11-17 13:23 PST, Jonathan Bedard	no flags	Details Formatted Diff Diff
Patch (50.68 KB, patch) 2017-11-27 15:18 PST, Jonathan Bedard	jbedard: review- jbedard: commit-queue-	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2017-11-10 10:59:59 PST

<rdar://problem/35472080>

Jonathan Bedard

Comment 2 2017-11-10 11:07:42 PST

Created attachment 326608 [details] Patch

Jonathan Bedard

Comment 3 2017-11-10 11:12:29 PST

This change will happen in 3 different patches. The first (posted now) is the meat of the change, the ServerProcess will no longer use any port bits. The second will allow the ServerProcess to be constructed with either a port or a host. The third will removed the support for constructing a ServerProcess with a port. We need to distinguish between the 2nd and 3rd patch because syncing up apple_additions and webkitpy changes is quite difficult.

Jonathan Bedard

Comment 4 2017-11-10 11:48:20 PST

Created attachment 326612 [details] Patch

Jonathan Bedard

Comment 5 2017-11-10 12:26:28 PST

Created attachment 326614 [details] Patch

Jonathan Bedard

Comment 6 2017-11-16 09:52:25 PST

Comment on attachment 326614 [details] Patch I discussed this with Dan Bates on 11/14. He is of the opinion that the ServerProcess should only be used for layout tests. As I've looked into how we use the server process, I think there is a better way forward. Throughout webkitpy, we have a number of places we could use an asynchronous shell command. For example, Simon mentioned to me a few days ago that when we run sample after a layout test has hung, we block until the command has returned. There is no reason we need to block. To solve this problem, I think we should split the server process into an asynchronous subprocess call and the server process, which would use the asynchronous subprocess call along with a few lifetime management tools and callbacks.

Jonathan Bedard

Comment 7 2017-11-16 17:07:11 PST

Created attachment 327132 [details] Patch

Jonathan Bedard

Comment 8 2017-11-16 17:08:11 PST

(In reply to Jonathan Bedard from comment #7) > Created attachment 327132 [details] > Patch This patch needs to be polished and have unit tests, sending it through EWS to check that it doesn't break Windows.

Jonathan Bedard

Comment 9 2017-11-17 13:23:05 PST

Created attachment 327223 [details] Patch

Jonathan Bedard

Comment 10 2017-11-27 15:18:09 PST

Created attachment 327694 [details] Patch

Jonathan Bedard

Comment 11 2017-12-04 10:11:44 PST

This is the wrong way to do this. The POpen object achieves nearly the same thing, and since we already embed a popen call in the executive, this code needlessly complicates the ServerProcess.

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