Minor cleanups in HttpStreamFactoryImpl and member classes.

* Remove all unnecessary IsSpdyAlternative() checks.
  IsSpdyAlternative() implies origin_url_.SchemeIs(url::kHttpsScheme),
  see DCHECK in constructor.
* Inline now const |spdy_certificate_error_|.
* Inline and remove SwitchToSpdyMode().  This method was only used twice out of
  four potential places.  Removal makes sense as the method is trivial, and
  there are no corresponding methods for |using_spdy_ = false|, |using_quic_ =
  true|, and |using_quic_ = false| assignments.
* Use url_constants instead of string literals for schemes (http, https, ftp,
  and wss).  There has already been two uses of such constants.
* Remove unused |destination| local variable.
* Fix some comments.
* git cl lint: Remove unnecessary semicolon, add includes.

BUG=475060

Review-Url: https://codereview.chromium.org/2359153003
Cr-Commit-Position: refs/heads/master@{#420725}

net/http/http_stream_factory_impl.cc

@@ -21,6 +21,7 @@
 #include "net/spdy/bidirectional_stream_spdy_impl.h"
 #include "net/spdy/spdy_http_stream.h"
 #include "url/gurl.h"
+#include "url/url_constants.h"
 
 namespace net {
 
@@ -218,7 +219,8 @@ void HttpStreamFactoryImpl::OnNewSpdySessionReady(
           used_ssl_config, used_proxy_info,
           new BidirectionalStreamSpdyImpl(spdy_session));
     } else {
-      bool use_relative_url = direct || request->url().SchemeIs("https");
+      bool use_relative_url =
+          direct || request->url().SchemeIs(url::kHttpsScheme);
       request->OnStreamReady(
           used_ssl_config, used_proxy_info,
           new SpdyHttpStream(spdy_session, use_relative_url));

net/http/http_stream_factory_impl_job.h

@@ -6,6 +6,7 @@
 #define NET_HTTP_HTTP_STREAM_FACTORY_IMPL_JOB_H_
 
 #include <memory>
+#include <utility>
 
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
@@ -257,23 +258,19 @@ class HttpStreamFactoryImpl::Job {
     STATE_RESOLVE_PROXY,
     STATE_RESOLVE_PROXY_COMPLETE,
 
-    // Note that when Alternate-Protocol says we can connect to an alternate
-    // port using a different protocol, we have the choice of communicating over
-    // the original protocol, or speaking the alternate protocol (currently,
-    // only npn-spdy) over an alternate port. For a cold page load, the http
-    // connection that delivers the http response that has the
-    // Alternate-Protocol header will already be warm. So, blocking the next
-    // http request on establishing a new npn-spdy connection would incur extra
-    // latency. Even if the http connection was not reused, establishing a new
-    // http connection is typically faster than npn-spdy, since npn-spdy
-    // requires a SSL handshake. Therefore, we start both the http and the
-    // npn-spdy jobs in parallel. In order not to unnecessarily waste sockets,
-    // we have the http job block on the npn-spdy job after proxy resolution.
-    // The npn-spdy job will Resume() the http job if, in
-    // STATE_INIT_CONNECTION_COMPLETE, it detects an error or does not find an
-    // existing SpdySession. In that case, the http and npn-spdy jobs will race.
-    // When QUIC protocol is used by the npn-spdy job, then http job will wait
-    // for |wait_time_| when the http job was resumed.
+    // The main and alternative jobs are started in parallel.  The main job
+    // waits after it finishes proxy resolution.  The alternative job never
+    // waits.
+    //
+    // An HTTP/2 alternative job notifies the JobController in DoInitConnection
+    // unless it can pool to an existing SpdySession.  JobController, in turn,
+    // resumes the main job.
+    //
+    // A QUIC alternative job notifies the JobController in DoInitConnection
+    // regardless of whether it pools to an existing QUIC session, but the main
+    // job is only resumed after some delay.
+    //
+    // If the main job is resumed, then it races the alternative job.
     STATE_WAIT,
     STATE_WAIT_COMPLETE,
 
@@ -376,9 +373,6 @@ class HttpStreamFactoryImpl::Job {
   // Called to handle a client certificate request.
   int HandleCertificateRequest(int error);
 
-  // Moves this stream request into SPDY mode.
-  void SwitchToSpdyMode();
-
   // Should we force QUIC for this stream request.
   bool ShouldForceQuic() const;
 
@@ -449,9 +443,6 @@ class HttpStreamFactoryImpl::Job {
   // Force quic for a specific port.
   int force_quic_port_;
 
-  // The certificate error while using SPDY over SSL for insecure URLs.
-  int spdy_certificate_error_;
-
   scoped_refptr<HttpAuthController>
       auth_controllers_[HttpAuth::AUTH_NUM_TARGETS];
 

net/http/http_stream_factory_impl_job_controller.cc

@@ -4,6 +4,10 @@
 
 #include "net/http/http_stream_factory_impl_job_controller.h"
 
+#include <memory>
+#include <string>
+#include <utility>
+
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
@@ -15,6 +19,7 @@
 #include "net/log/net_log_event_type.h"
 #include "net/proxy/proxy_server.h"
 #include "net/spdy/spdy_session.h"
+#include "url/url_constants.h"
 
 namespace net {
 
@@ -642,7 +647,7 @@ void HttpStreamFactoryImpl::JobController::CreateJobs(
              << alternative_service.host_port_pair().host()
              << " port: " << alternative_service.host_port_pair().port() << ")";
 
-    DCHECK(!request_info.url.SchemeIs("ftp"));
+    DCHECK(!request_info.url.SchemeIs(url::kFtpScheme));
     HostPortPair alternative_destination(alternative_service.host_port_pair());
     ignore_result(
         ApplyHostMappingRules(request_info.url, &alternative_destination));
@@ -877,7 +882,7 @@ HttpStreamFactoryImpl::JobController::GetAlternativeServiceForInternal(
     HttpStreamRequest::StreamType stream_type) {
   GURL original_url = request_info.url;
 
-  if (!original_url.SchemeIs("https"))
+  if (!original_url.SchemeIs(url::kHttpsScheme))
     return AlternativeService();
 
   url::SchemeHostPort origin(original_url);
@@ -954,7 +959,7 @@ HttpStreamFactoryImpl::JobController::GetAlternativeServiceForInternal(
     if (session_->quic_stream_factory()->IsQuicDisabled())
       continue;
 
-    if (!original_url.SchemeIs("https"))
+    if (!original_url.SchemeIs(url::kHttpsScheme))
       continue;
 
     // Check whether there is an existing QUIC session to use for this origin.