Fix loading success.html at the end of chrome://chrome-signin flow

extensions/browser/url_request_util.cc

 // Copyright 2014 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.
 
 #include "extensions/browser/url_request_util.h"
 
 #include <string>
 
 #include "content/public/browser/resource_request_info.h"
 #include "content/public/common/browser_side_navigation_policy.h"
@@ skipping 137 matching lines @@
 bool AllowCrossRendererResourceLoadHelper(bool is_guest,
                                           const Extension* extension,
                                           const Extension* owner_extension,
                                           const std::string& partition_id,
                                           const std::string& resource_path,
                                           ui::PageTransition page_transition,
                                           bool* allowed) {
   if (is_guest) {
     // An extension's resources should only be accessible to WebViews owned by
     // that extension.
-    if (owner_extension != extension) {
+    // The only exception is the sign-in extension loaded by
+    // chrome://chrome-signin, which is allowed to load web-accessible resources
+    // even if the webview that is loading the resource does not belog to the

[Charlie Reis, 2017/04/25 16:52:54]

nit: belong

[msarda, 2017/04/26 11:14:35]

Done.

+    // extension.
+    bool is_signin_extension =
+        extension && extension->id() == "mfffpogegjflfpflabcdkioaeobkgjik";

[Charlie Reis, 2017/04/25 16:52:54]

Is there a constant we can use here rather than a magic string?  Or is it in a
layer we can't access?

[msarda, 2017/04/26 11:14:35]

The constant is defined in
https://cs.chromium.org/chromium/src/chrome/browser/extensions/signin/gaia_au...
, but that is not accessible in //extensions/. I think having the string here
makes more sense than having a client/provider API as I do not think this should
be something configurable by the user of extensions.

+    if (owner_extension != extension && !is_signin_extension) {

[Charlie Reis, 2017/04/25 16:52:54]

Devlin: What do you think about whitelisting signin vs removing the whole
owner_extension check for now (which you suggested on the bug)?  I wasn't in
that review, so I'm not sure how important the check is.

[Devlin, 2017/04/26 01:59:37]

Lucas knows more about the history of this check than I do.  I was worried that
this might be something other apps are legitimately doing, but it seems like
maybe not?  If that's the case, I'm fine with a whitelist approach.

[msarda, 2017/04/26 11:14:34]

Lucas: Please advise on whether to keep this code as is or remove the it.

[lfg, 2017/04/26 16:24:18]

We should use the whitelist approach instead of removing the check. Removing the
owners check would mean that any app could load resources listed in the manifest
from any other app.

[Charlie Reis, 2017/04/26 20:10:25]

Acknowledged.

       *allowed = false;
       return true;
     }
 
     *allowed = WebviewInfo::IsResourceWebviewAccessible(extension, partition_id,
                                                         resource_path);
     return true;
   }
 
   return false;
 }
 
 }  // namespace url_request_util
 }  // namespace extensions