Removing x-x509-user-cert mime handler.

content/browser/loader/mime_type_resource_handler.cc

Index: content/browser/loader/mime_type_resource_handler.cc
diff --git a/content/browser/loader/mime_type_resource_handler.cc b/content/browser/loader/mime_type_resource_handler.cc
index 59c2b5310660487b1a0ccdc7f9591da7ebcc14bc..699391d367d3e0f5f0ff464394c41a9db29aa08e 100644
--- a/content/browser/loader/mime_type_resource_handler.cc
+++ b/content/browser/loader/mime_type_resource_handler.cc
@@ -16,7 +16,6 @@
 #include "components/mime_util/mime_util.h"
 #include "content/browser/download/download_resource_handler.h"
 #include "content/browser/download/download_stats.h"
-#include "content/browser/loader/certificate_resource_handler.h"
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
 #include "content/browser/loader/resource_request_info_impl.h"
 #include "content/browser/loader/stream_resource_handler.h"
@@ -341,14 +340,15 @@ bool MimeTypeResourceHandler::SelectNextHandler(bool* defer) {
   ResourceRequestInfoImpl* info = GetRequestInfo();
   const std::string& mime_type = response_->head.mime_type;
 
-  if (mime_util::IsSupportedCertificateMimeType(mime_type)) {
-    // Install certificate file.
-    info->set_is_download(true);
-    scoped_ptr<ResourceHandler> handler(
-        new CertificateResourceHandler(request()));
-    return UseAlternateNextHandler(handler.Pass(), std::string());
+  // Add a Content-Disposition header for x-x509-user-cert files that don't
+  // have a Content-Disposition set to force a download with a .crt extension.
+  if (mime_type == "application/x-x509-user-cert" &&
+      !response_->head.headers->HasHeader("Content-Disposition")) {
+    response_->head.headers->AddHeader(
+        "Content-Disposition: attachment; filename=\"user.crt\"");

[Ryan Sleevi, 2015/12/08 02:38:37]

I'm still torn on this. Do we anticipate ever ripping off the bandaid here and
no longer synthesizing a C-D - and instead, expect the servers to?

If so, at what point?

My 'gut' is that we should remove this code when we remove <keygen>, in which
case, we should file a bug to track that, suitably forward-reaching, and add a
comment here to that effect

// https://crbug.com/foo - "Temporary" hack to handle servers that aren't
setting
// Content-Disposition and expecting the browser to automatically
// install certificates; this is being deprecated and will be removed
// in XXX.

[svaldez, 2015/12/08 15:43:39]

We could get rid of the filename part of this and then just have a check to
force download if we do end up adding the hack to get the certificate manager to
auto-populate the filename.

[svaldez, 2015/12/08 16:34:35]

Should this be a separate bug from the Keygen deprecation, or can we use the
same bug for tracking this?

[Ryan Sleevi, 2015/12/09 01:27:24]

Separate; good to keep concrete tasks as distinct bugs.

   }
 
+
   // Allow requests for object/embed tags to be intercepted as streams.
   if (info->GetResourceType() == content::RESOURCE_TYPE_OBJECT) {
     DCHECK(!info->allow_download());
@@ -371,8 +371,9 @@ bool MimeTypeResourceHandler::SelectNextHandler(bool* defer) {
 
   bool must_download = MustDownload();
   if (!must_download) {
-    if (mime_util::IsSupportedMimeType(mime_type))
+    if (mime_util::IsSupportedMimeType(mime_type)) {

[Ryan Sleevi, 2015/12/08 02:38:37]

braces are unnecessary here; keep it consistent with rest of file (and net/ code
in general)

[svaldez, 2015/12/08 15:43:39]

Done.

       return true;
+    }
 
     bool handled_by_plugin;
     if (!SelectPluginHandler(defer, &handled_by_plugin))