Removed local RefPtr objects created from PassRefPtr arguments.

third_party/WebKit/Source/platform/loader/fetch/CrossOriginAccessControl.cpp

 /*
  * Copyright (C) 2008 Apple Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 454 matching lines @@
           "' contains a username and password, which is disallowed for"
           " cross-origin requests.");
       return;
     }
     default:
       NOTREACHED();
   }
 }
 
 bool CrossOriginAccessControl::HandleRedirect(
-    PassRefPtr<SecurityOrigin> security_origin,
+    RefPtr<SecurityOrigin> security_origin,
     ResourceRequest& new_request,
     const ResourceResponse& redirect_response,
     StoredCredentials with_credentials,
     ResourceLoaderOptions& options,
     String& error_message) {
   // http://www.w3.org/TR/cors/#redirect-steps terminology:
   const KURL& last_url = redirect_response.Url();
   const KURL& new_url = new_request.Url();
 
-  RefPtr<SecurityOrigin> current_security_origin = security_origin;
-
-  RefPtr<SecurityOrigin> new_security_origin = current_security_origin;

[Yuta Kitamura, 2017/04/10 07:00:11]

I think the removal of new_security_origin causes a behavioral change,
specifically:
  * l.527 sets new_security_origin, without changing current_security_origin,
  * l.531 tests a condition based on current_security_origin,
  * l.533 uses new_security_origin.

I think those two variables are intentionally separated, so you probably
should keep this separation.

[Bugs Nash, 2017/04/11 01:47:51]

I'm not removing new_security_origin, I'm setting it to the passed RefPtr
instead of the local one (they are the same)
I don't see how this is a behavior change. Previously security_origin (argument)
was *only* used to set current_security_origin, then never used again. So having
both is redundant right? If I had of changed the argument to be called
current_security_origin and deleted the local RefPtr called
current_security_origin it should have no impact on behavior. This patch does
the same thing, except it also renames current_security_origin to
security_origin.

[Yuta Kitamura, 2017/04/11 05:49:39]

Ah I see, I now see you didn't change the behavior.

However, I'd propose renaming |security_origin| (in the function arguments)
to |current_security_origin|, since current_ and new_ are paired here,
and the code handles a tricky transition (current_ -> new_) situation.

+  RefPtr<SecurityOrigin> new_security_origin = security_origin;
 
   // TODO(tyoshino): This should be fixed to check not only the last one but
   // all redirect responses.
-  if (!current_security_origin->CanRequest(last_url)) {
+  if (!security_origin->CanRequest(last_url)) {
     // Follow http://www.w3.org/TR/cors/#redirect-steps
     CrossOriginAccessControl::RedirectStatus redirect_status =
         CrossOriginAccessControl::CheckRedirectLocation(new_url);
     if (redirect_status != kRedirectSuccess) {
       StringBuilder builder;
       builder.Append("Redirect from '");
       builder.Append(last_url.GetString());
       builder.Append("' has been blocked by CORS policy: ");
       CrossOriginAccessControl::RedirectErrorString(builder, redirect_status,
                                                     new_url);
       error_message = builder.ToString();
       return false;
     }
 
     // Step 5: perform resource sharing access check.
     CrossOriginAccessControl::AccessStatus cors_status =
         CrossOriginAccessControl::CheckAccess(
-            redirect_response, with_credentials, current_security_origin.Get());
+            redirect_response, with_credentials, security_origin.Get());
     if (cors_status != kAccessAllowed) {
       StringBuilder builder;
       builder.Append("Redirect from '");
       builder.Append(last_url.GetString());
       builder.Append("' has been blocked by CORS policy: ");
       CrossOriginAccessControl::AccessControlErrorString(
-          builder, cors_status, redirect_response,
-          current_security_origin.Get(), new_request.GetRequestContext());
+          builder, cors_status, redirect_response, security_origin.Get(),
+          new_request.GetRequestContext());
       error_message = builder.ToString();
       return false;
     }
 
     RefPtr<SecurityOrigin> last_origin = SecurityOrigin::Create(last_url);
     // Set request's origin to a globally unique identifier as specified in
     // the step 10 in https://fetch.spec.whatwg.org/#http-redirect-fetch.
     if (!last_origin->CanRequest(new_url)) {
       options.security_origin = SecurityOrigin::CreateUnique();
       new_security_origin = options.security_origin;
     }
   }
 
-  if (!current_security_origin->CanRequest(new_url)) {
+  if (!security_origin->CanRequest(new_url)) {
     new_request.ClearHTTPOrigin();
     new_request.SetHTTPOrigin(new_security_origin.Get());
 
     // Unset credentials flag if request's credentials mode is "same-origin" as
     // request's response tainting becomes "cors".
     //
     // This is equivalent to the step 2 in
     // https://fetch.spec.whatwg.org/#http-network-or-cache-fetch
     if (options.credentials_requested == kClientDidNotRequestCredentials)
       options.allow_credentials = kDoNotAllowStoredCredentials;
   }
   return true;
 }
 
 }  // namespace blink