Remove HttpStreamFactoryImpl::Job::MarkAsAlternate.

Remove HttpStreamFactoryImpl::Job::MarkAsAlternate.

* Add AlternativeService argument to Job::Job() to signal whether Job is
  alternative;
* remove HttpStreamFactoryImpl::Job::MarkAsAlternate() method;
* add const bool is_spdy_alternative_ and is_quic_alternative members;
* remove Job::IsAlternate() and IsSpdyAlternate() methods;
* s/alternate/alternative/ in comments.

BUG=489280
Committed: https://crrev.com/6298e99674ce5f570255eb6301db7d32839f0b18
Cr-Commit-Position: refs/heads/master@{#332820}

[Bence, 2015-06-01 19:12:42]

bnc@chromium.org changed reviewers:
+ rch@chromium.org

[Bence, 2015-06-01 19:12:42]

Ryan: PTAL.  Thanks.

[Ryan Hamilton, 2015-06-02 17:26:57]

https://codereview.chromium.org/1162893003/diff/1/net/http/http_stream_factor...
File net/http/http_stream_factory_impl_job.h (right):

https://codereview.chromium.org/1162893003/diff/1/net/http/http_stream_factor...
net/http/http_stream_factory_impl_job.h:39: // A Job is considered alternative
iff
eschew "iff.  This reads just as unambiguiously when written as:

// A Job is considered alternative if 
// alternative_service.protocol != UNINITIALIZED_ALTERNATE_PROTOCOL.

That being said, since this class doesn't really describe hat it means to be
"considered alternative" I would be more inclined to say something which simply
documents how this argument is used.

That being said, I think it might be clearer to have two different constructor
forms. One which takes an AlternateService and one which does not. You can then
use c++ 11's delegated constructor, I believe to make this simple.

https://codereview.chromium.org/1162893003/diff/1/net/http/http_stream_factor...
net/http/http_stream_factory_impl_job.h:320: const bool is_quic_alternative_;
It's not obvious to me that this is a win, but perhaps it is. So this adds to
the memory used by each job (albeit not very much) and denormalizes information
out of alternative_service_ into these two new members. There's nothing *wrong*
with this of course, but I don't know that it's better?

[Bence, 2015-06-02 19:32:16]

PTAL, thanks.

https://codereview.chromium.org/1162893003/diff/1/net/http/http_stream_factor...
File net/http/http_stream_factory_impl_job.h (right):

https://codereview.chromium.org/1162893003/diff/1/net/http/http_stream_factor...
net/http/http_stream_factory_impl_job.h:39: // A Job is considered alternative
iff
On 2015/06/02 17:26:56, Ryan Hamilton wrote:
> eschew "iff.  This reads just as unambiguiously when written as:
> 
> // A Job is considered alternative if 
> // alternative_service.protocol != UNINITIALIZED_ALTERNATE_PROTOCOL.
> 
> That being said, since this class doesn't really describe hat it means to be
> "considered alternative" I would be more inclined to say something which
simply
> documents how this argument is used.
> 
> That being said, I think it might be clearer to have two different constructor
> forms. One which takes an AlternateService and one which does not. You can
then
> use c++ 11's delegated constructor, I believe to make this simple.

Great idea, thanks.

Done.

https://codereview.chromium.org/1162893003/diff/1/net/http/http_stream_factor...
net/http/http_stream_factory_impl_job.h:320: const bool is_quic_alternative_;
On 2015/06/02 17:26:57, Ryan Hamilton wrote:
> It's not obvious to me that this is a win, but perhaps it is. So this adds to
> the memory used by each job (albeit not very much) and denormalizes
information
> out of alternative_service_ into these two new members. There's nothing
*wrong*
> with this of course, but I don't know that it's better?

I agree: it doesn't seem to matter much.  I reverted to methods for lack of
compelling reason to use members.

Done.

[Ryan Hamilton, 2015-06-03 23:33:59]

lgtm

Thanks!

[Bence, 2015-06-04 11:13:57]

The CQ bit was checked by bnc@chromium.org

[commit-bot: I haz the power, 2015-06-04 11:14:20]

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

[commit-bot: I haz the power, 2015-06-04 12:17:56]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-06-04 12:19:39]

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