[chromedriver] Implement DELETE NetworkConditions

[chromedriver] Implement DELETE NetworkConditions

BUG=

Committed: https://crrev.com/b234648cb7ab94b8c2830b566f03dfd02ebab191
Cr-Commit-Position: refs/heads/master@{#322043}

[srawlins, 2015-03-18 17:47:19]

srawlins@google.com changed reviewers:
+ samuong@chromium.org

[samuong, 2015-03-18 17:58:43]

I get a conflict in run_py_tests.py when trying to patch this into my checkout.
Maybe it's time for a rebase?

https://codereview.chromium.org/1022543002/diff/1/chrome/test/chromedriver/te...
File chrome/test/chromedriver/test/run_py_tests.py (right):

https://codereview.chromium.org/1022543002/diff/1/chrome/test/chromedriver/te...
chrome/test/chromedriver/test/run_py_tests.py:820: self.assertEquals(False,
network['offline']);
should this return a zeroed-out dictionary, or a "network conditions must be set
before it can be retrieved" error?

[srawlins, 2015-03-18 18:24:25]

Sorry, I've rebased this CL.

https://codereview.chromium.org/1022543002/diff/1/chrome/test/chromedriver/te...
File chrome/test/chromedriver/test/run_py_tests.py (right):

https://codereview.chromium.org/1022543002/diff/1/chrome/test/chromedriver/te...
chrome/test/chromedriver/test/run_py_tests.py:820: self.assertEquals(False,
network['offline']);
On 2015/03/18 17:58:43, samuong wrote:
> should this return a zeroed-out dictionary, or a "network conditions must be
set
> before it can be retrieved" error?

Yeah I can refactor this to delete the overriddenNetworkConditions in the
session.

What would you prefer? I'm fine with... an empty dictionary. I think that might
be friendliest.

[samuong, 2015-03-18 18:32:24]

https://codereview.chromium.org/1022543002/diff/1/chrome/test/chromedriver/te...
File chrome/test/chromedriver/test/run_py_tests.py (right):

https://codereview.chromium.org/1022543002/diff/1/chrome/test/chromedriver/te...
chrome/test/chromedriver/test/run_py_tests.py:820: self.assertEquals(False,
network['offline']);
On 2015/03/18 18:24:25, srawlins wrote:
> On 2015/03/18 17:58:43, samuong wrote:
> > should this return a zeroed-out dictionary, or a "network conditions must be
> set
> > before it can be retrieved" error?
> 
> Yeah I can refactor this to delete the overriddenNetworkConditions in the
> session.
> 
> What would you prefer? I'm fine with... an empty dictionary. I think that
might
> be friendliest.

I'm OK with either, but I think that if a user performs the following steps, it
should return the same thing in steps 2 and 5:

1. create session
2. get network conditions
3. set network conditions to something else
4. delete network conditions
5. get network conditions

If I'm reading correctly, ExecuteGetNetworkConditions() currently returns an
error in this case?

[srawlins, 2015-03-23 18:12:50]

On 2015/03/18 18:32:24, samuong wrote:
>
https://codereview.chromium.org/1022543002/diff/1/chrome/test/chromedriver/te...
> File chrome/test/chromedriver/test/run_py_tests.py (right):
> 
>
https://codereview.chromium.org/1022543002/diff/1/chrome/test/chromedriver/te...
> chrome/test/chromedriver/test/run_py_tests.py:820: self.assertEquals(False,
> network['offline']);
> On 2015/03/18 18:24:25, srawlins wrote:
> > On 2015/03/18 17:58:43, samuong wrote:
> > > should this return a zeroed-out dictionary, or a "network conditions must
be
> > set
> > > before it can be retrieved" error?
> > 
> > Yeah I can refactor this to delete the overriddenNetworkConditions in the
> > session.
> > 
> > What would you prefer? I'm fine with... an empty dictionary. I think that
> might
> > be friendliest.
> 
> I'm OK with either, but I think that if a user performs the following steps,
it
> should return the same thing in steps 2 and 5:
> 
> 1. create session
> 2. get network conditions
> 3. set network conditions to something else
> 4. delete network conditions
> 5. get network conditions
> 
> If I'm reading correctly, ExecuteGetNetworkConditions() currently returns an
> error in this case?

Good logic, Sam. I've opted to raise an exception in each case. This feels much
more "correct", even if it is a little less friendly.

[samuong, 2015-03-23 20:23:46]

https://codereview.chromium.org/1022543002/diff/40001/chrome/test/chromedrive...
File chrome/test/chromedriver/window_commands.cc (right):

https://codereview.chromium.org/1022543002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/window_commands.cc:940: NetworkConditions*
network_conditions = new NetworkConditions();
Won't this leak? Why not allocate on the stack?

[srawlins, 2015-03-24 00:20:50]

On 2015/03/23 20:23:46, samuong wrote:
>
https://codereview.chromium.org/1022543002/diff/40001/chrome/test/chromedrive...
> File chrome/test/chromedriver/window_commands.cc (right):
> 
>
https://codereview.chromium.org/1022543002/diff/40001/chrome/test/chromedrive...
> chrome/test/chromedriver/window_commands.cc:940: NetworkConditions*
> network_conditions = new NetworkConditions();
> Won't this leak? Why not allocate on the stack?

Done.

[samuong, 2015-03-24 03:11:21]

Thanks! LGTM.

[srawlins, 2015-03-24 18:47:14]

The CQ bit was checked by srawlins@google.com

[commit-bot: I haz the power, 2015-03-24 18:47:42]

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

[commit-bot: I haz the power, 2015-03-24 19:19:51]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-03-24 19:21:02]

Patchset 4 (id:??) landed as
https://crrev.com/b234648cb7ab94b8c2830b566f03dfd02ebab191
Cr-Commit-Position: refs/heads/master@{#322043}