[Abandoned] Add Autofill to the <webview> chrome://chrome-signin page

chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.cc

Index: chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.cc
diff --git a/chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.cc b/chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.cc
index 89e6dae808efc3a05d7d43febc2ba398487c7154..080cb74b8be804a7d8fd6710d267a7a7213bf6df 100644
--- a/chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.cc
+++ b/chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.cc
@@ -2,15 +2,19 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-
 #include "chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.h"
 
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/chrome_extension_web_contents_observer.h"
 #include "chrome/browser/favicon/favicon_tab_helper.h"
+#include "chrome/browser/password_manager/chrome_password_manager_client.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/renderer_context_menu/render_view_context_menu.h"
+#include "chrome/browser/ui/autofill/chrome_autofill_client.h"
 #include "chrome/browser/ui/pdf/chrome_pdf_web_contents_helper_client.h"
 #include "chrome/common/chrome_version_info.h"
+#include "chrome/common/url_constants.h"
+#include "components/autofill/content/browser/content_autofill_driver_factory.h"
 #include "components/pdf/browser/pdf_web_contents_helper.h"
 #include "components/renderer_context_menu/context_menu_delegate.h"
 #include "extensions/browser/api/web_request/web_request_api.h"
@@ -76,6 +80,25 @@ void ChromeWebViewGuestDelegate::OnAttachWebViewHelpers(
       contents,
       scoped_ptr<pdf::PDFWebContentsHelperClient>(
           new ChromePDFWebContentsHelperClient()));
+
+  // <webview> in Chrome Apps and Extensions does not handle autofill.
+  if (web_view_guest_->in_extension())
+    return;
+
+  // Currently the Chrome sign in page is the only page that uses <webview>.
+  // If this changes in the future, revisit the use of autofill.
+  DCHECK(web_view_guest_->GetOwnerSiteURL().GetOrigin().spec() ==

[engedy, 2015/02/18 13:34:27]

@Joel, given that there is security implications of this check failing, should
we make it a CHECK(), as opposed to a DCHECK()? Or simply return at this point
if the condition is not met?

[Fady Samuel, 2015/02/18 14:08:20]

I have another maybe insignificant concern: this is doing a string comparison,
is it preferred to do a GURL comparison?

So:

DCHECK(web_view_guest_->GetOwnerSiteURL().GetOrigin() ==
GURL(chrome::kChromeUIChromeSigninURL));

?

[noms (inactive), 2015/02/18 18:33:12]

I've made it a hard return (this was my original suggestion, so I'm happy with
it)

[jww, 2015/02/18 20:08:45]

I think engedy is right, it should be a CHECK(), not a DCHECK, and I don't think
a hard return is the right behavior. If it hits this point, something has gone
terribly wrong in our code, so we should abort, and since it's a security issue,
it should be a run time failure.

I think the string comparison is fine since it's just a straight equality check
and we do care about the path.

[noms (inactive), 2015/02/18 21:04:03]

I don't think I understand how this is a security issue. A CHECK would mean that
only Chrome signin can use webview, which I think is too strict. The original
concern was that we were adding autofill to all of <webview>, which should go
through a feature launch. Adding autofill to *just* the signin page was ok,
which is exactly what this CL is doing. 

+      chrome::kChromeUIChromeSigninURL);
+
+  autofill::ChromeAutofillClient::CreateForWebContents(contents);
+  ChromePasswordManagerClient::CreateForWebContentsWithAutofillClient(

[engedy, 2015/02/18 13:34:27]

Could you please add a TODO that this will need to be revised later? I share
Vaclav's concern that currently the Password Manager does support actually
support this use-case, hence the solution is very fragile.

[engedy, 2015/02/18 13:35:28]

... does not support ...

[noms (inactive), 2015/02/18 18:33:12]

Done.

+      contents,
+      autofill::ChromeAutofillClient::FromWebContents(contents));
+  autofill::ContentAutofillDriverFactory::CreateForWebContentsAndDelegate(
+      contents,
+      autofill::ChromeAutofillClient::FromWebContents(contents),
+      g_browser_process->GetApplicationLocale(),
+      autofill::AutofillManager::ENABLE_AUTOFILL_DOWNLOAD_MANAGER);
 }
 
 void ChromeWebViewGuestDelegate::OnDidCommitProvisionalLoadForFrame(