GTTF: Convert most tests in content to use EmbeddedTestServer.

GTTF: Convert most tests in content to use EmbeddedTestServer.

This eliminates a whole class of problems with SpawnedTestServer
failing to start.

BUG=96594
R=avi@chromium.org, satorux@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=202112

[Avi (use Gerrit), 2013-05-21 00:24:22]

lgtm

https://codereview.chromium.org/15505003/diff/1/net/test/embedded_test_server...
File net/test/embedded_test_server/http_response.h (right):

https://codereview.chromium.org/15505003/diff/1/net/test/embedded_test_server...
net/test/embedded_test_server/http_response.h:28: // Respresents a HTTP
response.
fix typo

[satorux1, 2013-05-21 07:40:18]

https://codereview.chromium.org/15505003/diff/1/net/test/embedded_test_server...
File net/test/embedded_test_server/http_response.h (right):

https://codereview.chromium.org/15505003/diff/1/net/test/embedded_test_server...
net/test/embedded_test_server/http_response.h:33: virtual std::string
ToResponseString() const = 0;
function comment is missing. maybe:

// Returns a HTTP response which starts from "HTTP/x.x" line,
// followed by response headers like "Content-Type: text/html".

https://codereview.chromium.org/15505003/diff/1/net/test/embedded_test_server...
net/test/embedded_test_server/http_response.h:36: class HttpResponseImpl :
public HttpResponse {
I found this name unusual. Usually classes named "Impl" are implementation
details thus not used as a public API (they are often defined in .cc file).

Maybe:

// Interface for HTTP response implementations.
class HttpResponseInterface {
  ...

  virtual std::string ToResponseString() const = 0;
}

// This class is used to handle basic HTTP responses with commonly used
// response headers such as "Content-Type".
class HttpResponse : public HttpResponseInterface {
  ...
}

This way, you don't have to touch the existing code.

[Paweł Hajdan Jr., 2013-05-21 17:56:03]

PTAL

https://codereview.chromium.org/15505003/diff/1/net/test/embedded_test_server...
File net/test/embedded_test_server/http_response.h (right):

https://codereview.chromium.org/15505003/diff/1/net/test/embedded_test_server...
net/test/embedded_test_server/http_response.h:28: // Respresents a HTTP
response.
On 2013/05/21 00:24:22, Avi wrote:
> fix typo

Done.

https://codereview.chromium.org/15505003/diff/1/net/test/embedded_test_server...
net/test/embedded_test_server/http_response.h:33: virtual std::string
ToResponseString() const = 0;
On 2013/05/21 07:40:18, satorux1 wrote:
> function comment is missing. maybe:
> 
> // Returns a HTTP response which starts from "HTTP/x.x" line,
> // followed by response headers like "Content-Type: text/html".

Updated. Note that one of the implementations deliberately returns an empty
string there to trigger ERR_EMPTY_RESPONSE, so my actual comment reflects that.

https://codereview.chromium.org/15505003/diff/1/net/test/embedded_test_server...
net/test/embedded_test_server/http_response.h:36: class HttpResponseImpl :
public HttpResponse {
On 2013/05/21 07:40:18, satorux1 wrote:
> I found this name unusual. Usually classes named "Impl" are implementation
> details thus not used as a public API (they are often defined in .cc file).
> 
> Maybe:
> 
> // Interface for HTTP response implementations.
> class HttpResponseInterface {
>   ...
> 
>   virtual std::string ToResponseString() const = 0;
> }
> 
> // This class is used to handle basic HTTP responses with commonly used
> // response headers such as "Content-Type".
> class HttpResponse : public HttpResponseInterface {
>   ...
> }
> 
> This way, you don't have to touch the existing code. 

Done.

[satorux1, 2013-05-22 01:54:56]

LGTM with a request:

https://codereview.chromium.org/15505003/diff/6001/net/test/embedded_test_ser...
File net/test/embedded_test_server/embedded_test_server.cc (right):

https://codereview.chromium.org/15505003/diff/6001/net/test/embedded_test_ser...
net/test/embedded_test_server/embedded_test_server.cc:171: if (!request_handled)
{
thank you for fixing the bug.

https://codereview.chromium.org/15505003/diff/6001/net/test/embedded_test_ser...
File net/test/embedded_test_server/http_response.h (right):

https://codereview.chromium.org/15505003/diff/6001/net/test/embedded_test_ser...
net/test/embedded_test_server/http_response.h:41: class HttpResponse : public
HttpResponseInterface {
I suggested this name, but I changed my mind (sorry about this!). I think the
followings are nicer:

class HttpResponse { ...
}
class BasicHttpResponse : public HttpResponse {
}

What do you think? This is similar to your original code, but the derived class
name is BasicHttpResponse, instead of HttpResponseImpl

[Paweł Hajdan Jr., 2013-05-22 21:01:38]

https://codereview.chromium.org/15505003/diff/6001/net/test/embedded_test_ser...
File net/test/embedded_test_server/http_response.h (right):

https://codereview.chromium.org/15505003/diff/6001/net/test/embedded_test_ser...
net/test/embedded_test_server/http_response.h:41: class HttpResponse : public
HttpResponseInterface {
On 2013/05/22 01:54:56, satorux1 wrote:
> I suggested this name, but I changed my mind (sorry about this!). I think the
> followings are nicer:
> 
> class HttpResponse { ...
> }
> class BasicHttpResponse : public HttpResponse {
> }
> 
> What do you think? This is similar to your original code, but the derived
class
> name is BasicHttpResponse, instead of HttpResponseImpl

Done.

[Paweł Hajdan Jr., 2013-05-22 23:22:30]

PTAL, there are some non-trivial changes here.

[satorux1, 2013-05-23 01:03:23]

LGTM

[Paweł Hajdan Jr., 2013-05-24 16:53:42]

Committed patchset #6 manually as r202112.