Move make_javascript_deterministic to UserStory.

Move make_javascript_deterministic to UserStory.

BUG=439512, 439700
Committed: https://crrev.com/1ea2aacffca6ea3474085d6b61a470086665d2da
Cr-Commit-Position: refs/heads/master@{#308707}

[chrishenry, 2014-12-13 20:06:29]

Patchset #1 (id:1) has been deleted

[chrishenry, 2014-12-13 20:06:55]

chrishenry@google.com changed reviewers:
+ nednguyen@google.com, slamm@chromium.org

[nednguyen, 2014-12-14 02:53:54]

lgtm

https://codereview.chromium.org/806513002/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/page/shared_page_state.py (right):

https://codereview.chromium.org/806513002/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/page/shared_page_state.py:106:
browser_options.extra_wpr_args, make_javascript_deterministic)
@slamm: does network_controller knows to restart wpr_server if
make_javascript_deterministic value changes? I remembered that currently, this
value can not be changed dynamically once the server is on.

[nednguyen, 2014-12-14 02:54:15]

On 2014/12/14 02:53:54, nednguyen wrote:
> lgtm
> 
>
https://codereview.chromium.org/806513002/diff/20001/tools/telemetry/telemetr...
> File tools/telemetry/telemetry/page/shared_page_state.py (right):
> 
>
https://codereview.chromium.org/806513002/diff/20001/tools/telemetry/telemetr...
> tools/telemetry/telemetry/page/shared_page_state.py:106:
> browser_options.extra_wpr_args, make_javascript_deterministic)
> @slamm: does network_controller knows to restart wpr_server if
> make_javascript_deterministic value changes? I remembered that currently, this
> value can not be changed dynamically once the server is on.

err, not lgtm yet. My mistake.

[chrishenry - DO NOT USE, 2014-12-14 03:54:27]

On 2014/12/14 02:54:15, nednguyen wrote:
> On 2014/12/14 02:53:54, nednguyen wrote:
> > lgtm
> > 
> >
>
https://codereview.chromium.org/806513002/diff/20001/tools/telemetry/telemetr...
> > File tools/telemetry/telemetry/page/shared_page_state.py (right):
> > 
> >
>
https://codereview.chromium.org/806513002/diff/20001/tools/telemetry/telemetr...
> > tools/telemetry/telemetry/page/shared_page_state.py:106:
> > browser_options.extra_wpr_args, make_javascript_deterministic)
> > @slamm: does network_controller knows to restart wpr_server if
> > make_javascript_deterministic value changes? I remembered that currently,
this
> > value can not be changed dynamically once the server is on.
> 
> err, not lgtm yet. My mistake.

I don't believe this is the case. We restarted wpr server on every archive
update, at least that's how I understand it from previous conversation and
existing network_controller_backend code. Eventually, we wanted to have a
persistent server that is updatable via an IPC command (either via subprocess
pipe or a http request to wpr server), but this isn't the case today.

[nednguyen, 2014-12-14 16:08:05]

On 2014/12/14 03:54:27, chrishenry - DO NOT USE wrote:
> On 2014/12/14 02:54:15, nednguyen wrote:
> > On 2014/12/14 02:53:54, nednguyen wrote:
> > > lgtm
> > > 
> > >
> >
>
https://codereview.chromium.org/806513002/diff/20001/tools/telemetry/telemetr...
> > > File tools/telemetry/telemetry/page/shared_page_state.py (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/806513002/diff/20001/tools/telemetry/telemetr...
> > > tools/telemetry/telemetry/page/shared_page_state.py:106:
> > > browser_options.extra_wpr_args, make_javascript_deterministic)
> > > @slamm: does network_controller knows to restart wpr_server if
> > > make_javascript_deterministic value changes? I remembered that currently,
> this
> > > value can not be changed dynamically once the server is on.
> > 
> > err, not lgtm yet. My mistake.
> 
> I don't believe this is the case. We restarted wpr server on every archive
> update, at least that's how I understand it from previous conversation and
> existing network_controller_backend code. Eventually, we wanted to have a
> persistent server that is updatable via an IPC command (either via subprocess
> pipe or a http request to wpr server), but this isn't the case today.

Ok, it looks like slamm@ compared the arguments dictionary to decide whether to
restart. One question though, I wonder whether we will ever want to set this
field to False in telemetry.

[slamm, 2014-12-15 19:08:09]

https://codereview.chromium.org/806513002/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/page/shared_page_state.py (right):

https://codereview.chromium.org/806513002/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/page/shared_page_state.py:106:
browser_options.extra_wpr_args, make_javascript_deterministic)
On 2014/12/14 02:53:54, nednguyen wrote:
> @slamm: does network_controller knows to restart wpr_server if
> make_javascript_deterministic value changes? I remembered that currently, this
> value can not be changed dynamically once the server is on.

It looks like you already figured this out. Yes, the network_controller restarts
WPR if any SetReplayArgs parameters change from one call to the next.

(FYI, before the network controller, I believe telemetry only restarted WPR if
the archive path changed.)

[nednguyen, 2014-12-15 19:09:07]

On 2014/12/15 19:08:09, slamm wrote:
>
https://codereview.chromium.org/806513002/diff/20001/tools/telemetry/telemetr...
> File tools/telemetry/telemetry/page/shared_page_state.py (right):
> 
>
https://codereview.chromium.org/806513002/diff/20001/tools/telemetry/telemetr...
> tools/telemetry/telemetry/page/shared_page_state.py:106:
> browser_options.extra_wpr_args, make_javascript_deterministic)
> On 2014/12/14 02:53:54, nednguyen wrote:
> > @slamm: does network_controller knows to restart wpr_server if
> > make_javascript_deterministic value changes? I remembered that currently,
this
> > value can not be changed dynamically once the server is on.
> 
> It looks like you already figured this out. Yes, the network_controller
restarts
> WPR if any SetReplayArgs parameters change from one call to the next.
> 
> (FYI, before the network controller, I believe telemetry only restarted WPR if
> the archive path changed.)

I think if there is no reason for make_javascript_deterministic to be False, we
should just remove this param.

[slamm, 2014-12-15 19:13:14]

lgtm

[chrishenry, 2014-12-16 01:32:51]

On 2014/12/15 19:09:07, nednguyen wrote:
> On 2014/12/15 19:08:09, slamm wrote:
> >
>
https://codereview.chromium.org/806513002/diff/20001/tools/telemetry/telemetr...
> > File tools/telemetry/telemetry/page/shared_page_state.py (right):
> > 
> >
>
https://codereview.chromium.org/806513002/diff/20001/tools/telemetry/telemetr...
> > tools/telemetry/telemetry/page/shared_page_state.py:106:
> > browser_options.extra_wpr_args, make_javascript_deterministic)
> > On 2014/12/14 02:53:54, nednguyen wrote:
> > > @slamm: does network_controller knows to restart wpr_server if
> > > make_javascript_deterministic value changes? I remembered that currently,
> this
> > > value can not be changed dynamically once the server is on.
> > 
> > It looks like you already figured this out. Yes, the network_controller
> restarts
> > WPR if any SetReplayArgs parameters change from one call to the next.
> > 
> > (FYI, before the network controller, I believe telemetry only restarted WPR
if
> > the archive path changed.)
> 
> I think if there is no reason for make_javascript_deterministic to be False,
we
> should just remove this param.

Is because we use it a sufficiently strong reason? d: See our own internal page
set. There was some timing bug that Ari discovered requiring us to disable JS
determinism. I don't think we want JS determinism for GWS at first.

[nednguyen, 2014-12-16 02:02:13]

ok. lgtm

[chrishenry, 2014-12-17 00:03:58]

The CQ bit was checked by chrishenry@google.com

[commit-bot: I haz the power, 2014-12-17 00:05:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/806513002/40001

[commit-bot: I haz the power, 2014-12-17 00:14:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-17 00:14:15]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[chrishenry, 2014-12-17 00:16:08]

chrishenry@google.com changed reviewers:
+ kbr@chromium.org

[chrishenry, 2014-12-17 00:16:08]

+kbr for content/test/gpu

[Ken Russell (switch to Gerrit), 2014-12-17 00:45:27]

content/test/gpu lgtm

[chrishenry, 2014-12-17 00:50:49]

The CQ bit was checked by chrishenry@google.com

[commit-bot: I haz the power, 2014-12-17 00:51:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/806513002/40001

[commit-bot: I haz the power, 2014-12-17 01:18:38]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2014-12-17 01:19:28]

Patchset 2 (id:??) landed as
https://crrev.com/1ea2aacffca6ea3474085d6b61a470086665d2da
Cr-Commit-Position: refs/heads/master@{#308707}