[Smart Lock] Smart Lock branding in save password infobar available only for signed users.

chrome/browser/password_manager/save_password_infobar_delegate.cc

Index: chrome/browser/password_manager/save_password_infobar_delegate.cc
diff --git a/chrome/browser/password_manager/save_password_infobar_delegate.cc b/chrome/browser/password_manager/save_password_infobar_delegate.cc
index 54bfc2814b4cda4d4d6104c8ccd45432262508fe..449f5cbf78bd61cc4086789945ef7dfefc460309 100644
--- a/chrome/browser/password_manager/save_password_infobar_delegate.cc
+++ b/chrome/browser/password_manager/save_password_infobar_delegate.cc
@@ -6,6 +6,8 @@
 
 #include "base/metrics/histogram.h"
 #include "chrome/browser/infobars/infobar_service.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/passwords/password_bubble_experiment.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/infobars/core/infobar.h"
@@ -38,8 +40,8 @@ void SavePasswordInfoBarDelegate::Create(
   InfoBarService* infobar_service =
       InfoBarService::FromWebContents(web_contents);
   SavePasswordInfoBarDelegate* infobar_delegate =
-      new SavePasswordInfoBarDelegate(
-          form_to_save.Pass(), uma_histogram_suffix, source_type);
+      new SavePasswordInfoBarDelegate(web_contents, form_to_save.Pass(),
+                                      uma_histogram_suffix, source_type);
 #if defined(OS_ANDROID)
   // For Android in case of smart lock we need different appearance of infobar.
   scoped_ptr<infobars::InfoBar> infobar =
@@ -79,6 +81,7 @@ SavePasswordInfoBarDelegate::~SavePasswordInfoBarDelegate() {
 }
 
 SavePasswordInfoBarDelegate::SavePasswordInfoBarDelegate(
+    content::WebContents* web_contents,
     scoped_ptr<password_manager::PasswordFormManager> form_to_save,
     const std::string& uma_histogram_suffix,
     password_manager::CredentialSourceType source_type)
@@ -86,7 +89,8 @@ SavePasswordInfoBarDelegate::SavePasswordInfoBarDelegate(
       form_to_save_(form_to_save.Pass()),
       infobar_response_(password_manager::metrics_util::NO_RESPONSE),
       uma_histogram_suffix_(uma_histogram_suffix),
-      source_type_(source_type) {
+      source_type_(source_type),
+      web_contents_(web_contents) {
   if (!uma_histogram_suffix_.empty()) {
     password_manager::metrics_util::LogUMAHistogramBoolean(
         "PasswordManager.SavePasswordPromptDisplayed_" + uma_histogram_suffix_,
@@ -125,11 +129,14 @@ void SavePasswordInfoBarDelegate::InfoBarDismissed() {
 }
 
 base::string16 SavePasswordInfoBarDelegate::GetMessageText() const {
-  int brand_string_id =
-      (source_type_ ==
-       password_manager::CredentialSourceType::CREDENTIAL_SOURCE_API)
-          ? IDS_PASSWORD_MANAGER_SMART_LOCK
-          : IDS_SAVE_PASSWORD_TITLE_BRAND;
+  int brand_string_id = IDS_SAVE_PASSWORD_TITLE_BRAND;
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents_->GetBrowserContext());
+  if (source_type_ ==
+          password_manager::CredentialSourceType::CREDENTIAL_SOURCE_API ||

[vabr (Chromium), 2015/05/11 15:29:10]

Did you mean && instead of ||?
(At least that's what the description says.)

[Mike West, 2015/05/11 15:51:48]

Moreover, why are we changing the branding based on whether or not the API is
being used? It goes into the same storage, and has the same properties, right?
I'd suggest that this should just be toggled based on the
`IsEnabledSmartLockBranding` bit, rather than on the source type.

[melandory, 2015/05/12 09:07:23]

I meant || here. Because my understanding was that in future we'll show Smart
Lock branding for every user  and my idea was that infobar which I pop up on API
call represents the "final" version of infobar (title and buttons). But I asked
sabine yesterday and she confirmed that I got it wrong. So I've remove
CREDENTIAL_SOURCE_API here completely.

And an addition, despite I fixed title, infobar for API calls still be different
(it has another buttons set), the idea is that the final design is not there and
infobar which is shown on api calls represents the latest version, but since
it's not approved by UX yet we are showing it only for API calls.

+      password_bubble_experiment::IsEnabledSmartLockBranding(profile)) {
+    brand_string_id = IDS_PASSWORD_MANAGER_SMART_LOCK;
+  }
   return l10n_util::GetStringFUTF16(IDS_SAVE_PASSWORD,
                                     l10n_util::GetStringUTF16(brand_string_id));
 }