Removed local RefPtr objects created from PassRefPtr arguments.

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

Index: third_party/WebKit/Source/platform/loader/fetch/CrossOriginAccessControl.cpp
diff --git a/third_party/WebKit/Source/platform/loader/fetch/CrossOriginAccessControl.cpp b/third_party/WebKit/Source/platform/loader/fetch/CrossOriginAccessControl.cpp
index 06d0234b67c6af8bcbd42bf367d70bd62bff85d5..82ad4097745ddb699a3af723877ca650e37b025d 100644
--- a/third_party/WebKit/Source/platform/loader/fetch/CrossOriginAccessControl.cpp
+++ b/third_party/WebKit/Source/platform/loader/fetch/CrossOriginAccessControl.cpp
@@ -472,7 +472,7 @@ void CrossOriginAccessControl::RedirectErrorString(
 }
 
 bool CrossOriginAccessControl::HandleRedirect(
-    PassRefPtr<SecurityOrigin> security_origin,
+    RefPtr<SecurityOrigin> security_origin,
     ResourceRequest& new_request,
     const ResourceResponse& redirect_response,
     StoredCredentials with_credentials,
@@ -482,13 +482,11 @@ bool CrossOriginAccessControl::HandleRedirect(
   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);
@@ -506,15 +504,15 @@ bool CrossOriginAccessControl::HandleRedirect(
     // 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;
     }
@@ -528,7 +526,7 @@ bool CrossOriginAccessControl::HandleRedirect(
     }
   }
 
-  if (!current_security_origin->CanRequest(new_url)) {
+  if (!security_origin->CanRequest(new_url)) {
     new_request.ClearHTTPOrigin();
     new_request.SetHTTPOrigin(new_security_origin.Get());