SSLUITest's work with ephemeral port testserver.

SSLUITest's work with ephemeral port testserver.

Many of the tests depend on a file served from one server to reference resources on a different server to exercise mixed content warnings.

To handle this, I added replace_orig and replace_new query parameters to /files/ based paths which do simple string substitution.

BUG=56814
TEST=browser_tests --gtest_filter="*SSLUI*"

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=65311

[cbentzel, 2010-11-03 22:02:20]

I'll remove some of the additional urlparser usage in testserver into a separate
CL to minimize overlap. Need to take off now though, so wanted you to look at
this.

http://codereview.chromium.org/4429001/diff/1/8
File net/tools/testserver/testserver.py (right):

http://codereview.chromium.org/4429001/diff/1/8#newcode439
net/tools/testserver/testserver.py:439: _, _, _, _, query, _ =
urlparse.urlparse(self.path)
Remove from this CL - separate change

[Paweł Hajdan Jr., 2010-11-04 07:29:30]

I have not commented testserver.py yet, I'm just not sure which hunks are parts
of which change. A general comment: src and dst do not make it obvious it's for
text replacement. Maybe find_text and replace_with, or something like that?

http://codereview.chromium.org/4429001/diff/1/2
File chrome/browser/ssl/ssl_browser_tests.cc (right):

http://codereview.chromium.org/4429001/diff/1/2#newcode120
chrome/browser/ssl/ssl_browser_tests.cc:120: std::string GetReplacementPath(
nit: I'd prefer a name like FillWithHostAndPort and something like that.
"Replacement path" is very vague.

http://codereview.chromium.org/4429001/diff/1/2#newcode160
chrome/browser/ssl/ssl_browser_tests.cc:160: const std::string replacement_path
= GetReplacementPath(
nit: Either follow the style guide for constants (kReplacementPath), or remove
const. I'm fine with either.

[cbentzel, 2010-11-04 20:40:18]

I isolated changes in testserver.py to file handler only, and changed the vague
"src" and "dst" query parameters to "replace_orig" and "replace_new".

http://codereview.chromium.org/4429001/diff/1/2
File chrome/browser/ssl/ssl_browser_tests.cc (right):

http://codereview.chromium.org/4429001/diff/1/2#newcode120
chrome/browser/ssl/ssl_browser_tests.cc:120: std::string GetReplacementPath(
On 2010/11/04 07:29:31, Paweł Hajdan Jr. wrote:
> nit: I'd prefer a name like FillWithHostAndPort and something like that.
> "Replacement path" is very vague.

Done.

http://codereview.chromium.org/4429001/diff/1/2#newcode160
chrome/browser/ssl/ssl_browser_tests.cc:160: const std::string replacement_path
= GetReplacementPath(
On 2010/11/04 07:29:31, Paweł Hajdan Jr. wrote:
> nit: Either follow the style guide for constants (kReplacementPath), or remove
> const. I'm fine with either.

The style guide mentions kReplacementPath format for compile-time constants
only, no mention of run-time constants.

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Constant_Names

(you have to expand the arrow to see)

[Paweł Hajdan Jr., 2010-11-05 14:58:06]

LGTM with nits.

http://codereview.chromium.org/4429001/diff/1/2
File chrome/browser/ssl/ssl_browser_tests.cc (right):

http://codereview.chromium.org/4429001/diff/1/2#newcode160
chrome/browser/ssl/ssl_browser_tests.cc:160: const std::string replacement_path
= GetReplacementPath(
On 2010/11/04 20:40:19, cbentzel wrote:
> On 2010/11/04 07:29:31, Paweł Hajdan Jr. wrote:
> > nit: Either follow the style guide for constants (kReplacementPath), or
remove
> > const. I'm fine with either.
> 
> The style guide mentions kReplacementPath format for compile-time constants
> only, no mention of run-time constants.
> 
> http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Constant_Names

Hehe, okay. Probably not worth to argue, but still - could you just remove the
const? I think that would be more consistent with rest of the code.

http://codereview.chromium.org/4429001/diff/6001/7007
File net/tools/testserver/testserver.py (right):

http://codereview.chromium.org/4429001/diff/6001/7007#newcode580
net/tools/testserver/testserver.py:580: a new string is returned with all
occasions of the 'src' value replaced
nit: Update these parts of the comments too. There is no src and dst now.

http://codereview.chromium.org/4429001/diff/6001/7007#newcode581
net/tools/testserver/testserver.py:581: by the 'dst value. This is a simple
string substitution, not a structured
nit: "This is a simple string substitution, not a structured replacement." seems
a bit confusing to me. I don't know what "structured replacement" would mean. I
suggest just removing that sentence.