Implement the BadHTTPSFallback Integration Test with ChromeDriver

Implement the BadHTTPSFallback Integration Test with ChromeDriver

BUG=680553
Review-Url: https://codereview.chromium.org/2813703005
Cr-Commit-Position: refs/heads/master@{#464165}
Committed: https://chromium.googlesource.com/chromium/src/+/d55f29611fd97ec16b512afede3f3a4d70173cd8

[bustamante, 2017-04-11 22:04:09]

bustamante@chromium.org changed reviewers:
+ bustamante@chromium.org

[bustamante, 2017-04-11 22:04:10]

looks good, except for a couple comments.

https://codereview.chromium.org/2813703005/diff/1/tools/chrome_proxy/webdrive...
File tools/chrome_proxy/webdriver/bypass.py (right):

https://codereview.chromium.org/2813703005/diff/1/tools/chrome_proxy/webdrive...
tools/chrome_proxy/webdriver/bypass.py:180: # set the primary (https) proxy to a
bad one.
Nit - the "S" in set should be capitalized.

https://codereview.chromium.org/2813703005/diff/1/tools/chrome_proxy/webdrive...
tools/chrome_proxy/webdriver/bypass.py:192: self.assertEqual(u'http/1.1',
response.protocol)
Port 80 may be a better check as it could also go over http/1.2, http/2, or
quic.

[jiewu, 2017-04-11 23:01:01]

Patchset #2 (id:20001) has been deleted

[jiewu, 2017-04-11 23:09:38]

On 2017/04/11 22:04:10, bustamante wrote:
> looks good, except for a couple comments.
> 
>
https://codereview.chromium.org/2813703005/diff/1/tools/chrome_proxy/webdrive...
> File tools/chrome_proxy/webdriver/bypass.py (right):
> 
>
https://codereview.chromium.org/2813703005/diff/1/tools/chrome_proxy/webdrive...
> tools/chrome_proxy/webdriver/bypass.py:180: # set the primary (https) proxy to
a
> bad one.
> Nit - the "S" in set should be capitalized.
> 
>
https://codereview.chromium.org/2813703005/diff/1/tools/chrome_proxy/webdrive...
> tools/chrome_proxy/webdriver/bypass.py:192: self.assertEqual(u'http/1.1',
> response.protocol)
> Port 80 may be a better check as it could also go over http/1.2, http/2, or
> quic.

Thank you for your comments. I have updated accordingly.

[bustamante, 2017-04-11 23:15:18]

bustamante@chromium.org changed reviewers:
+ ryansturm@chromium.org

[bustamante, 2017-04-11 23:15:18]

lgtm thanks!  Adding Ryan

https://codereview.chromium.org/2813703005/diff/40001/tools/chrome_proxy/webd...
File tools/chrome_proxy/webdriver/bypass.py (right):

https://codereview.chromium.org/2813703005/diff/40001/tools/chrome_proxy/webd...
tools/chrome_proxy/webdriver/bypass.py:184:
test_driver.AddChromeArg('--data-reduction-proxy-http-proxies='
Nit - trailing whitespace

[RyanSturm, 2017-04-12 19:54:31]

lgtm % a few comments. The comments apply to a few places throughout the change

https://codereview.chromium.org/2813703005/diff/40001/tools/chrome_proxy/webd...
File tools/chrome_proxy/webdriver/bypass.py (right):

https://codereview.chromium.org/2813703005/diff/40001/tools/chrome_proxy/webd...
tools/chrome_proxy/webdriver/bypass.py:176: # a http proxy
nit: s/http/HTTP/ s/https/HTTPS/ s/proxy/proxy./ s/DataSaver/Data Saver/

https://codereview.chromium.org/2813703005/diff/40001/tools/chrome_proxy/webd...
tools/chrome_proxy/webdriver/bypass.py:191: self.assertEqual(80, response.port)
If you add "self.assertHasChromeProxyViaHeader(response)" does it pass? It would
be good to have if so.

[jiewu, 2017-04-12 20:44:24]

On 2017/04/12 19:54:31, Ryan Sturm wrote:
> lgtm % a few comments. The comments apply to a few places throughout the
change
> 
>
https://codereview.chromium.org/2813703005/diff/40001/tools/chrome_proxy/webd...
> File tools/chrome_proxy/webdriver/bypass.py (right):
> 
>
https://codereview.chromium.org/2813703005/diff/40001/tools/chrome_proxy/webd...
> tools/chrome_proxy/webdriver/bypass.py:176: # a http proxy
> nit: s/http/HTTP/ s/https/HTTPS/ s/proxy/proxy./ s/DataSaver/Data Saver/
> 
>
https://codereview.chromium.org/2813703005/diff/40001/tools/chrome_proxy/webd...
> tools/chrome_proxy/webdriver/bypass.py:191: self.assertEqual(80,
response.port)
> If you add "self.assertHasChromeProxyViaHeader(response)" does it pass? It
would
> be good to have if so.

Thank you for your comments. I have updated accordingly. 

"If you add "self.assertHasChromeProxyViaHeader(response)" does it pass? It
would be good to have if so."
I did and it did not pass. I reported this to Richard yesterday.

[jiewu, 2017-04-12 20:48:12]

Patchset #5 (id:100001) has been deleted

[jiewu, 2017-04-12 20:49:36]

Patchset #5 (id:120001) has been deleted

[jiewu, 2017-04-12 20:52:20]

The CQ bit was checked by jiewu@chromium.org

[jiewu, 2017-04-12 20:52:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
bustamante@chromium.org, ryansturm@chromium.org
Link to the patchset: https://codereview.chromium.org/2813703005/#ps140001
(title: "Remove white space")

[commit-bot: I haz the power, 2017-04-12 20:52:58]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-12 22:01:03]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1492030340173220,
"parent_rev": "41a618265465c164db0083511143b0bab5cdc697", "commit_rev":
"d55f29611fd97ec16b512afede3f3a4d70173cd8"}

[commit-bot: I haz the power, 2017-04-12 22:02:02]

Description was changed from

==========
Implement the BadHTTPSFallback Integration Test with ChromeDriver

BUG=680553
==========

to

==========
Implement the BadHTTPSFallback Integration Test with ChromeDriver

BUG=680553

Review-Url: https://codereview.chromium.org/2813703005
Cr-Commit-Position: refs/heads/master@{#464165}
Committed:
https://chromium.googlesource.com/chromium/src/+/d55f29611fd97ec16b512afede3f...
==========

[commit-bot: I haz the power, 2017-04-12 22:02:02]

Committed patchset #5 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/d55f29611fd97ec16b512afede3f...