sandwich: Adds command line flag to disable WPR injections

sandwich: Adds command line flag to disable WPR injections

By default, the Web Page Replay server automatically inject a
javascript (deterministic.js) file that overrides Math.random()
and Date() to implementations that return a deterministic value.

BUG=582080
Committed: https://crrev.com/9d7185f35638545a782d62619d5d73a9ac44552c
Cr-Commit-Position: refs/heads/master@{#375569}

[gabadie, 2016-02-15 11:44:34]

gabadie@chromium.org changed reviewers:
+ lizeb@chromium.org, mattcary@chromium.org, pasko@chromium.org

[gabadie, 2016-02-15 11:44:34]

lizeb@chromium.org & mattcary@chromium.org: PTAL.

[mattcary, 2016-02-15 12:42:28]

lgtm

https://codereview.chromium.org/1698883002/diff/1/tools/android/loading/devic...
File tools/android/loading/device_setup.py (right):

https://codereview.chromium.org/1698883002/diff/1/tools/android/loading/devic...
tools/android/loading/device_setup.py:124: record: Enables or disables WPR
archive recording.
add js_determinism to the args section.

nit: it might be more clear to rename the param disable_script_injection to
match what you're doing with the WPR flag and straightenout the currently
somewhat twisted logic below. This will also be future proof in case WPR starts
doing more than just js_determinism when injecting scripts.

ie, the code below might read more clearly like:
if disable_script_injection:
  # Remove default WPR injected scripts like deterministic.js
  # which overrides Math.random
  wpr_server_args.extend(['--inject_scripts', ''])

https://codereview.chromium.org/1698883002/diff/1/tools/android/loading/run_s...
File tools/android/loading/run_sandwich.py (right):

https://codereview.chromium.org/1698883002/diff/1/tools/android/loading/run_s...
tools/android/loading/run_sandwich.py:297:
parser.add_argument('--wpr-nojsdeterminism', default=False,
As discussed at greater length in the other file, --disable_wpr_script_injection
or something similar might be more clear.

[gabadie, 2016-02-15 15:33:09]

New revision addressing matt's nits uploaded.

lizeb@chromium.org: PTAL.

https://codereview.chromium.org/1698883002/diff/1/tools/android/loading/devic...
File tools/android/loading/device_setup.py (right):

https://codereview.chromium.org/1698883002/diff/1/tools/android/loading/devic...
tools/android/loading/device_setup.py:124: record: Enables or disables WPR
archive recording.
On 2016/02/15 12:42:28, mattcary wrote:
> add js_determinism to the args section.
> 
> nit: it might be more clear to rename the param disable_script_injection to
> match what you're doing with the WPR flag and straightenout the currently
> somewhat twisted logic below. This will also be future proof in case WPR
starts
> doing more than just js_determinism when injecting scripts.
> 
> ie, the code below might read more clearly like:
> if disable_script_injection:
>   # Remove default WPR injected scripts like deterministic.js
>   # which overrides Math.random
>   wpr_server_args.extend(['--inject_scripts', ''])

Good point. Done.

https://codereview.chromium.org/1698883002/diff/1/tools/android/loading/run_s...
File tools/android/loading/run_sandwich.py (right):

https://codereview.chromium.org/1698883002/diff/1/tools/android/loading/run_s...
tools/android/loading/run_sandwich.py:297:
parser.add_argument('--wpr-nojsdeterminism', default=False,
On 2016/02/15 12:42:28, mattcary wrote:
> As discussed at greater length in the other file,
--disable_wpr_script_injection
> or something similar might be more clear.

Done.

[Benoit L, 2016-02-15 15:49:53]

On 2016/02/15 15:33:09, gabadie wrote:
> New revision addressing matt's nits uploaded.
> 
> mailto:lizeb@chromium.org: PTAL.
> 
>
https://codereview.chromium.org/1698883002/diff/1/tools/android/loading/devic...
> File tools/android/loading/device_setup.py (right):
> 
>
https://codereview.chromium.org/1698883002/diff/1/tools/android/loading/devic...
> tools/android/loading/device_setup.py:124: record: Enables or disables WPR
> archive recording.
> On 2016/02/15 12:42:28, mattcary wrote:
> > add js_determinism to the args section.
> > 
> > nit: it might be more clear to rename the param disable_script_injection to
> > match what you're doing with the WPR flag and straightenout the currently
> > somewhat twisted logic below. This will also be future proof in case WPR
> starts
> > doing more than just js_determinism when injecting scripts.
> > 
> > ie, the code below might read more clearly like:
> > if disable_script_injection:
> >   # Remove default WPR injected scripts like deterministic.js
> >   # which overrides Math.random
> >   wpr_server_args.extend(['--inject_scripts', ''])
> 
> Good point. Done.
> 
>
https://codereview.chromium.org/1698883002/diff/1/tools/android/loading/run_s...
> File tools/android/loading/run_sandwich.py (right):
> 
>
https://codereview.chromium.org/1698883002/diff/1/tools/android/loading/run_s...
> tools/android/loading/run_sandwich.py:297:
> parser.add_argument('--wpr-nojsdeterminism', default=False,
> On 2016/02/15 12:42:28, mattcary wrote:
> > As discussed at greater length in the other file,
> --disable_wpr_script_injection
> > or something similar might be more clear.
> 
> Done.

lgtm

[gabadie, 2016-02-15 16:38:02]

Description was changed from

==========
sandwich: Adds command line flag to disable Javascript determinism

By default, the Web Page Replay server automatically inject a
javascript (deterministic.js) file that overrides Math.random()
and Date() to implementations that return a deterministic value.

BUG=582080
==========

to

==========
sandwich: Adds command line flag to disable WPR injections
    
By default, the Web Page Replay server automatically inject a
javascript (deterministic.js) file that overrides Math.random()
and Date() to implementations that return a deterministic value.

BUG=582080
==========

[gabadie, 2016-02-16 14:07:21]

The CQ bit was checked by gabadie@chromium.org

[gabadie, 2016-02-16 14:07:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
mattcary@chromium.org
Link to the patchset: https://codereview.chromium.org/1698883002/#ps20001
(title: "Address matt's nits")

[commit-bot: I haz the power, 2016-02-16 14:07:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1698883002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1698883002/20001

[commit-bot: I haz the power, 2016-02-16 14:39:14]

Description was changed from

==========
sandwich: Adds command line flag to disable WPR injections
    
By default, the Web Page Replay server automatically inject a
javascript (deterministic.js) file that overrides Math.random()
and Date() to implementations that return a deterministic value.

BUG=582080
==========

to

==========
sandwich: Adds command line flag to disable WPR injections
    
By default, the Web Page Replay server automatically inject a
javascript (deterministic.js) file that overrides Math.random()
and Date() to implementations that return a deterministic value.

BUG=582080
==========

[commit-bot: I haz the power, 2016-02-16 14:39:16]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-02-16 22:52:39]

Description was changed from

==========
sandwich: Adds command line flag to disable WPR injections
    
By default, the Web Page Replay server automatically inject a
javascript (deterministic.js) file that overrides Math.random()
and Date() to implementations that return a deterministic value.

BUG=582080
==========

to

==========
sandwich: Adds command line flag to disable WPR injections

By default, the Web Page Replay server automatically inject a
javascript (deterministic.js) file that overrides Math.random()
and Date() to implementations that return a deterministic value.

BUG=582080

Committed: https://crrev.com/9d7185f35638545a782d62619d5d73a9ac44552c
Cr-Commit-Position: refs/heads/master@{#375569}
==========

[commit-bot: I haz the power, 2016-02-16 22:52:40]

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