Get rid of net::DO_NOT_PROMPT_FOR_LOGIN

content/browser/loader/resource_dispatcher_host_impl.cc

Index: content/browser/loader/resource_dispatcher_host_impl.cc
diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc
index 6d1aaddae129b7cb288759ba2c16cb28f1e989af..324372b81ecc977c59dcc95d948696d127dd2464 100644
--- a/content/browser/loader/resource_dispatcher_host_impl.cc
+++ b/content/browser/loader/resource_dispatcher_host_impl.cc
@@ -595,6 +595,7 @@ DownloadInterruptReason ResourceDispatcherHostImpl::BeginDownload(
     int child_id,
     int route_id,
     bool prefer_cache,
+    bool do_not_prompt_for_login,
     scoped_ptr<DownloadSaveInfo> save_info,
     uint32 download_id,
     const DownloadStartedCallback& started_callback) {
@@ -657,6 +658,7 @@ DownloadInterruptReason ResourceDispatcherHostImpl::BeginDownload(
 
   ResourceRequestInfoImpl* extra_info =
       CreateRequestInfo(child_id, route_id, true, context);
+  extra_info->set_do_not_prompt_for_login(do_not_prompt_for_login);
   extra_info->AssociateWithRequest(request.get());  // Request takes ownership.
 
   if (request->url().SchemeIs(url::kBlobScheme)) {
@@ -1192,8 +1194,6 @@ void ResourceDispatcherHostImpl::BeginRequest(
   headers.AddHeadersFromString(request_data.headers);
   new_request->SetExtraRequestHeaders(headers);
 
-  new_request->SetLoadFlags(load_flags);
-
   storage::BlobStorageContext* blob_context =
       GetBlobStorageContext(filter_->blob_storage_context());
   // Resolve elements from request_body and prepare upload data.
@@ -1219,6 +1219,26 @@ void ResourceDispatcherHostImpl::BeginRequest(
 
   bool allow_download = request_data.allow_download &&
       IsResourceTypeFrame(request_data.resource_type);

[mmenke, 2014/12/10 19:28:30]

Suggest moving the load_flags and is_sync_load stuff down here, too, to keep
related code together.  Also, the DCHECKs on what flags are set should probably
be just before the SetLoadFlags line, now that we're modifying flags here, too.

[baranovich, 2014/12/10 22:24:38]

Done.

+  bool do_not_prompt_for_login =
+      request_data.resource_type == RESOURCE_TYPE_PREFETCH ||
+      request_data.resource_type == RESOURCE_TYPE_FAVICON;

[mmenke, 2014/12/10 19:28:30]

BUG:  You need request_data.do_not_prompt_for_login in the mix, too.  The fact
this didn't make any tests fail is a bit concerning.

I think this may be a little clearer, than just adding another || clause:

bool do_not_prompt_for_login = request_data.do_not_prompt_for_login;
if (... || ...)
  do_not_prompt_for_login = true;

[baranovich, 2014/12/10 22:24:38]

I didn't found any tests (at least in RDH browsertests/unittests) on that
prompting/non-prompting issue btw.

[baranovich, 2014/12/10 22:24:38]

Done.

+  if (request_data.resource_type == RESOURCE_TYPE_IMAGE &&
+      HTTP_AUTH_RELATION_BLOCKED_CROSS ==
+          HttpAuthRelationTypeOf(request_data.url,
+                                 request_data.first_party_for_cookies)) {
+    // Prevent third-party image content from prompting for login, as this
+    // is often a scam to extract credentials for another domain from the user.
+    // Only block image loads, as the attack applies largely to the "src"
+    // property of the <img> tag. It is common for web properties to allow
+    // untrusted values for <img src>; this is considered a fair thing for an
+    // HTML sanitizer to do. Conversely, any HTML sanitizer that didn't
+    // filter sources for <script>, <link>, <embed>, <object>, <iframe> tags
+    // would be considered vulnerable in and of itself.
+    do_not_prompt_for_login = true;
+    load_flags |= net::LOAD_DO_NOT_USE_EMBEDDED_IDENTITY;

[baranovich, 2014/12/09 23:33:49]

not sure if it's the best decision. On the one hand, I want to keep the things
together, since they're together:) on the other hand, there is
BuildLoadFlagsForRequest() ti build request flags.

[mmenke, 2014/12/10 19:28:30]

It's certainly a bit ugly, but I don't have a better suggestion.

+  }
+
+  new_request->SetLoadFlags(load_flags);
 
   // Make extra info and read footer (contains request ID).
   ResourceRequestInfoImpl* extra_info =
@@ -1241,6 +1261,7 @@ void ResourceDispatcherHostImpl::BeginRequest(
           request_data.has_user_gesture,
           request_data.enable_load_timing,
           request_data.enable_upload_progress,
+          do_not_prompt_for_login,
           request_data.referrer_policy,
           request_data.visiblity_state,
           resource_context,
@@ -1486,9 +1507,9 @@ ResourceRequestInfoImpl* ResourceDispatcherHostImpl::CreateRequestInfo(
       0,
       request_id_,
       MSG_ROUTING_NONE,  // render_frame_id
-      false,     // is_main_frame
-      false,     // parent_is_main_frame
-      -1,        // parent_render_frame_id
+      false,             // is_main_frame
+      false,             // parent_is_main_frame
+      -1,                // parent_render_frame_id
       RESOURCE_TYPE_SUB_RESOURCE,
       ui::PAGE_TRANSITION_LINK,
       false,     // should_replace_current_entry
@@ -1498,6 +1519,7 @@ ResourceRequestInfoImpl* ResourceDispatcherHostImpl::CreateRequestInfo(
       false,     // has_user_gesture
       false,     // enable_load_timing
       false,     // enable_upload_progress
+      false,     // do_not_prompt_for_login
       blink::WebReferrerPolicyDefault,
       blink::WebPageVisibilityStateVisible,
       context,
@@ -1952,6 +1974,7 @@ void ResourceDispatcherHostImpl::BeginNavigationRequest(
           info.navigation_params.has_user_gesture,
           true,   // enable_load_timing
           false,  // enable_upload_progress
+          false,  // do_not_prompt_for_login
           params.referrer.policy,
           // TODO(davidben): This is only used for prerenders. Replace
           // is_showing with something for that. Or maybe it just comes from the
@@ -2326,24 +2349,7 @@ int ResourceDispatcherHostImpl::BuildLoadFlagsForRequest(
   } else if (request_data.resource_type == RESOURCE_TYPE_SUB_FRAME) {
     load_flags |= net::LOAD_SUB_FRAME;
   } else if (request_data.resource_type == RESOURCE_TYPE_PREFETCH) {
-    load_flags |= (net::LOAD_PREFETCH | net::LOAD_DO_NOT_PROMPT_FOR_LOGIN);
-  } else if (request_data.resource_type == RESOURCE_TYPE_FAVICON) {
-    load_flags |= net::LOAD_DO_NOT_PROMPT_FOR_LOGIN;
-  } else if (request_data.resource_type == RESOURCE_TYPE_IMAGE) {
-    // Prevent third-party image content from prompting for login, as this
-    // is often a scam to extract credentials for another domain from the user.
-    // Only block image loads, as the attack applies largely to the "src"
-    // property of the <img> tag. It is common for web properties to allow
-    // untrusted values for <img src>; this is considered a fair thing for an
-    // HTML sanitizer to do. Conversely, any HTML sanitizer that didn't
-    // filter sources for <script>, <link>, <embed>, <object>, <iframe> tags
-    // would be considered vulnerable in and of itself.
-    HttpAuthRelationType relation_type = HttpAuthRelationTypeOf(
-        request_data.url, request_data.first_party_for_cookies);
-    if (relation_type == HTTP_AUTH_RELATION_BLOCKED_CROSS) {
-      load_flags |= (net::LOAD_DO_NOT_USE_EMBEDDED_IDENTITY |
-                     net::LOAD_DO_NOT_PROMPT_FOR_LOGIN);
-    }
+    load_flags |= net::LOAD_PREFETCH;
   }
 
   if (is_sync_load)