1, 2, 3 sounds good to me.

(b), (c) probably don't really happen, I think.

For (b), I think they just want to close the existing WPR server & restart it (because we haven't implemented changing WPR's args on fly yet). Maybe all we need is to change it to call StopReplay().

I think about this a bit more & I think we should move the args in Open() method to StartReplay() method. So techically, it's collapse StartReplay() & Open()

The (b) I saw was in:
https://github.com/catapult-project/catapult/blob/37921f135dc61021b8ced4c73375339658ac9953/telemetry/telemetry/page/shared_page_state.py#L91

Where it looks more like a hacky statement of the sorts: "maybe we didn't close it properly before, and trying to open now will cause an error, so let's close it".

I think the correct behavior should be more like (a): if already open, check whether the new args are consistent with the previos ones, if yes: reuse the controller, if no: complain.

So agree (b) and (c) shouldn't happen and we should make all clients use the controller more like (a).

On #2: No: Open and InitializeIfNeeded both set options that are expected to hold for the entire session (e.g. wpr_mode should not change).

While StartReplay sets options that may change from one page/story to the next.

Actually, both of use were kind of right.

The extra_wpr_args that Open receives actually belong to StartReplay; not to Open nor InitializeIfNeeded. So that option should be moved to its right place.

On the other side, Open is really the name that makes most sense for clients to call to start proxy/forwarder etc.

So my proposal is broadly:

- Merge current Open + StartReplay -> StartReplay
- Rename InitializeIfNeeded to Open and keep with behavior (a)
- Delete existing Open. Which really does nothing in the current implementation.

Gladly most of the code and client callers live in catapult, so this is not such a painful change to make.

Getting a CL mostly ready to review.

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/d3b3c7243695e1fd611a6159091518c9b681c2d3

commit d3b3c7243695e1fd611a6159091518c9b681c2d3
Author: Juan Antonio Navarro Perez <perezju@google.com>
Date: Thu Nov 23 10:02:26 2017

[Telemetry] Cleanup network controller backend implementation

This change:
- Renames IntializeIfNeeded to Open.
- Allows Open to reuse existing servers if possible.
- Moves the extra_wpr_args from Open to StartReplay.
- Removes the old Open method (no longer needed).
- Adjusts all client calls as needed.

Also:
- Fix bugs in shared_state_unittest which were not calling
  TearDownState after each test, thus leaving open network
  controllers.

Bug:  chromium:786413 
Change-Id: I0931bef5059652fb0453cfebfc666248a158941b
Reviewed-on: https://chromium-review.googlesource.com/782219
Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>

[modify] https://crrev.com/d3b3c7243695e1fd611a6159091518c9b681c2d3/telemetry/telemetry/internal/backends/chrome/chrome_browser_backend.py
[modify] https://crrev.com/d3b3c7243695e1fd611a6159091518c9b681c2d3/telemetry/telemetry/internal/snap_page_util_unittest.py
[modify] https://crrev.com/d3b3c7243695e1fd611a6159091518c9b681c2d3/telemetry/telemetry/testing/serially_executed_browser_test_case.py
[modify] https://crrev.com/d3b3c7243695e1fd611a6159091518c9b681c2d3/telemetry/telemetry/core/local_server.py
[modify] https://crrev.com/d3b3c7243695e1fd611a6159091518c9b681c2d3/telemetry/telemetry/internal/platform/network_controller_backend.py
[modify] https://crrev.com/d3b3c7243695e1fd611a6159091518c9b681c2d3/telemetry/telemetry/testing/browser_test_case.py
[modify] https://crrev.com/d3b3c7243695e1fd611a6159091518c9b681c2d3/telemetry/telemetry/internal/platform/network_controller_backend_unittest.py
[modify] https://crrev.com/d3b3c7243695e1fd611a6159091518c9b681c2d3/telemetry/telemetry/page/shared_page_state.py
[modify] https://crrev.com/d3b3c7243695e1fd611a6159091518c9b681c2d3/telemetry/telemetry/core/network_controller.py
[modify] https://crrev.com/d3b3c7243695e1fd611a6159091518c9b681c2d3/telemetry/telemetry/internal/backends/chrome/chrome_browser_backend_unittest.py
[modify] https://crrev.com/d3b3c7243695e1fd611a6159091518c9b681c2d3/telemetry/telemetry/internal/browser/extension_unittest.py
[modify] https://crrev.com/d3b3c7243695e1fd611a6159091518c9b681c2d3/telemetry/telemetry/page/shared_page_state_unittest.py
[modify] https://crrev.com/d3b3c7243695e1fd611a6159091518c9b681c2d3/telemetry/telemetry/testing/fakes/__init__.py
[modify] https://crrev.com/d3b3c7243695e1fd611a6159091518c9b681c2d3/telemetry/telemetry/internal/browser/browser_unittest.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1b743d52155038b6a5ecc164b4098e45612d1584

commit 1b743d52155038b6a5ecc164b4098e45612d1584
Author: Juan Antonio Navarro Perez <perezju@google.com>
Date: Fri Nov 24 11:57:13 2017

[tools/perf] Switch clients to new network_controller API

Use the new API as exposed by:
https://chromium-review.googlesource.com/c/catapult/+/782219

TBR=nednguyen@google.com

Bug:  786413 
Change-Id: Ibc77a4e4538f5886e57542652476b2636315dfe8
Reviewed-on: https://chromium-review.googlesource.com/785673
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519088}
[modify] https://crrev.com/1b743d52155038b6a5ecc164b4098e45612d1584/tools/perf/page_sets/dual_browser_story.py
[modify] https://crrev.com/1b743d52155038b6a5ecc164b4098e45612d1584/tools/perf/profile_creators/profile_extender.py

+achuith, assigning to you to make the necessary changes in ChromeOs clients.

When that's done please assign back to me for the final cleanup.

https://chromium-review.googlesource.com/852355

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/245411bf95adad85f1444ccd95e7605e4f849fe0

commit 245411bf95adad85f1444ccd95e7605e4f849fe0
Author: Achuith Bhandarkar <achuith@chromium.org>
Date: Tue Jan 09 01:22:54 2018

[chrome]: Use new api Open instead of InitializeIfNeeded()

BUG= chromium:786413 
TEST=trybots

Change-Id: I3798c03d07e0f7a238f2a906393bd070fd206f18
Reviewed-on: https://chromium-review.googlesource.com/852355
Commit-Ready: Achuith Bhandarkar <achuith@chromium.org>
Tested-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>

[modify] https://crrev.com/245411bf95adad85f1444ccd95e7605e4f849fe0/client/common_lib/cros/chrome.py

Over to you, Juan

Thanks!

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/aaaa5510da3e1a9ce62b7731caab65f0a83e3cf6

commit aaaa5510da3e1a9ce62b7731caab65f0a83e3cf6
Author: Juan Antonio Navarro Perez <perezju@google.com>
Date: Tue Jan 09 12:00:33 2018

[Telemetry] Final network_controller cleanup

Remove the obsolete and no longer used:
- InitializeIfNeeded method.
- extra_wpr_args passed to the Open method.

Bug:  chromium:786413 
Change-Id: Ibe31d81f3298ddc07efe6f5ae270ce57e1a36a8d
Reviewed-on: https://chromium-review.googlesource.com/853861
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>

[modify] https://crrev.com/aaaa5510da3e1a9ce62b7731caab65f0a83e3cf6/telemetry/telemetry/core/network_controller.py