Common Name Mismatch Handler For WWW Subdomain Mismatch case

chrome/browser/ssl/common_name_mismatch_handler.h

+// Copyright (c) 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_SSL_COMMON_NAME_MISMATCH_HANDLER_H_
+#define CHROME_BROWSER_SSL_COMMON_NAME_MISMATCH_HANDLER_H_
+
+#include "base/basictypes.h"
+#include "base/callback.h"
+#include "base/compiler_specific.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/threading/non_thread_safe.h"
+#include "base/time/time.h"
+#include "net/url_request/url_fetcher.h"
+#include "net/url_request/url_fetcher_delegate.h"
+#include "net/url_request/url_request_context_getter.h"
+#include "url/gurl.h"
+
+// This class contains methods to get a suggested URL when certificate
+// validation fails due to |ERR_CERT_COMMON_NAME_INVALID| and has methods to
+// perform a network request to check the validity of new URL.
+class CommonNameMismatchHandler : public net::URLFetcherDelegate,
+                                  public base::NonThreadSafe {
+ public:
+  enum SuggestedUrlCheckResult {
+    // The request succeeds with good response code i.e. URL exists and its
+    // certificate is valid.
+    RESULT_SUGGESTED_URL_VALID,
+    // Suggested URL is invalid
+    RESULT_SUGGESTED_URL_INVALID

[meacer, 2015/07/15 20:11:46]

nit: rename to AVAILABLE/NOT_AVAILABLE instead of VALID/INVALID

You need to change the comments and test names as well.

[Bhanu Dev, 2015/07/16 23:38:05]

Using AVAILABLE/NOT_AVAILABLE is a good idea, I am also thinking there would be
more confusion between |suggested_url_exists| (to say that a suggested url
exists and can be checked if its valid) and |suggested_url_available| than with
|suggested_url_valid|. 

+  };
+
+  struct Results {
+    Results() : result(RESULT_SUGGESTED_URL_INVALID) {}
+
+    SuggestedUrlCheckResult result;
+    GURL new_url;

[meacer, 2015/07/15 20:11:46]

suggested_url?

[Bhanu Dev, 2015/07/16 23:38:05]

If there is a redirect, |new_url| can be different from that of |suggested_url|.
So, used a different name here.

+  };
+
+  typedef base::Callback<void(const Results& results)> CheckUrlCallback;
+
+  explicit CommonNameMismatchHandler(const GURL request_url,
+      const scoped_refptr<net::URLRequestContextGetter>& request_context);
+  ~CommonNameMismatchHandler() override;
+
+  // Triggers a check to validate suggested URL. After completion, runs the
+  // |callback|.
+  void CheckSuggestedUrl(const GURL& url,
+                         const CheckUrlCallback& CheckUrlcallback);

[palmer, 2015/07/16 22:55:32]

Nit: Name the argument |checkUrlCallback| (note capitalization) or, more simply,
|callback|.

[Bhanu Dev, 2015/07/16 23:38:05]

Done.

+
+  // Returns true if the error can be handled. Interacts with

[palmer, 2015/07/16 22:55:32]

Nit: Don't use the passive voice here ("...be handled"), because it lets you
leave out the actor (i.e. who is doing the handling). Say who does the handling,
and/or use a verb more specific than "handle".

E.g. "Returns true if any other DNS names in the certificate's subject
alternative names list closely match the original request hostname and if those
other names are available and serving valid certificates."

[Bhanu Dev, 2015/07/16 23:38:06]

Done. I just now realized that I use a lot of passive voice in documentation.
Thanks. I will take care of it, from now on. 

+  // |SslErrorClassification| to find the cause of the certificate
+  // validation error. If the error can be handled, writes a suggestion
+  // for a new URL to try into |suggested_url|.
+  static bool GetSuggestedUrl(const GURL& request_url,

[meacer, 2015/07/15 20:11:46]

I'd just return a GURL() here, with empty GURL meaning no suggested URL.

[palmer, 2015/07/16 22:55:32]

I agree.

[Bhanu Dev, 2015/07/16 23:38:06]

I thought that, returning bool makes it easy to check if suggested_url exists.
Like using |if(GetSuggestedUrl(...))| would be easier in this case.

Please let me know if you think the other case would be better. Thanks. 

[meacer, 2015/07/28 01:18:04]

I think it's one less dimension you'll need to worry about when you just return
a GURL. I always get confused thinking about possible combinations (4 in your
case vs 2 when returning GURL).

+                              const std::vector<std::string>& dns_names,
+                              GURL* suggested_url);
+
+ private:
+  // net::URLFetcherDelegate:
+  void OnURLFetchComplete(const net::URLFetcher* source) override;
+
+  // Takes a |net::URLFetcher| that has finished trying to retrieve the
+  // suggested URL, and fills a |Results| struct based on its result.
+  void GetSuggestedUrlCheckResult(const net::URLFetcher* url_fetcher,
+                                  Results* results) const;
+
+  const GURL request_url_;
+
+  // Returns true if the check is currently running.
+  bool CheckingSuggestedUrl() const;
+
+  scoped_refptr<net::URLRequestContextGetter> request_context_;
+
+  CheckUrlCallback check_url_callback_;
+
+  scoped_ptr<net::URLFetcher> url_fetcher_;
+
+  DISALLOW_COPY_AND_ASSIGN(CommonNameMismatchHandler);
+};
+
+#endif  // CHROME_BROWSER_SSL_COMMON_NAME_MISMATCH_HANDLER_H_