Shorten text next to checkmark. Use yellow text instead of an alert dialog.

See screen cap in bug.

BUG=41356

Review URL: http://codereview.chromium.org/1642006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@44417 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 76 insertions, 88 deletions

diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index 02f9eca..7885dbd 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -3303,15 +3303,9 @@ each locale. -->
       <message name="IDS_EXTENSION_UNINSTALL_PROMPT_TITLE" desc="Titlebar of the extension uninstallation prompt window">
         Confirm Uninstallation
       </message>
-      <message name="IDS_EXTENSION_ENABLE_INCOGNITO_PROMPT_TITLE" desc="Titlebar of the extension run-in-incognito prompt window">
-        Confirm Run in Incognito
-      </message>
       <message name="IDS_EXTENSION_INSTALL_PROMPT_HEADING" desc="First bold line in the content area of the extension installation prompt. Asks the user if they want to install a particular extension.">
         Install <ph name="EXTENSION_NAME">$1<ex>Gmail Checker</ex></ph>?
       </message>
-      <message name="IDS_EXTENSION_ENABLE_INCOGNITO_PROMPT_HEADING" desc="First bold line in the content area of the run-in-incognito prompt. Asks the user if they want to enable a particular extension in incognito.">
-        Allow <ph name="EXTENSION_NAME">$1<ex>Gmail Checker</ex></ph> to run in incognito?
-      </message>
       <message name="IDS_EXTENSION_PROMPT_CREATE_SHORTCUT" desc="Label for checkbox that asks the user if they want to create a desktop shortcut for an app install.">
         Create desktop shortcut
       </message>
@@ -3321,9 +3315,6 @@ each locale. -->
       <message name="IDS_EXTENSION_UNINSTALL_CONFIRMATION" desc="The warning you get when you are about to uninstall an extension.">
         Are you sure you want to uninstall this extension?
       </message>
-      <message name="IDS_EXTENSION_PROMPT_WARNING_INCOGNITO" desc="The warning you get when you are about to enable an extension in incognito.">
-        Are you sure you want to allow this extension to run in incognito? <ph name="PRODUCT_NAME">$1<ex>Google Chrome</ex></ph> cannot prevent this extension from recording your browsing data.
-      </message>
       <message name="IDS_EXTENSION_PROMPT_WARNING_BROWSER" desc="Warning displayed in body of extension dialog when the extension requires access to browser APIs.">
         This extension will have access to your browsing history.
       </message>
@@ -3536,7 +3527,10 @@ each locale. -->
         Enable
       </message>
       <message name="IDS_EXTENSIONS_ENABLE_INCOGNITO" desc="The checkbox for enabling extensions in incognito.">
-        Allow this extension to run in incognito
+        Allow in incognito
+      </message>
+      <message name="IDS_EXTENSIONS_INCOGNITO_WARNING" desc="Warns the user that Chrome cannot prevent extensions from recording history in incognito mode. Displayed in extensions management UI after an extension is selected to be run in incognito mode.">
+        <ph name="BEGIN_BOLD">&lt;b&gt;</ph>Warning:<ph name="END_BOLD">&lt;/b&gt;</ph> Google Chrome cannot prevent extensions from recording your browsing history. To disable this extension in incognito mode, unselect this option.
       </message>
       <message name="IDS_EXTENSIONS_RELOAD" desc="The link for reloading extensions.">
         Reload
diff --git a/chrome/browser/extensions/extension_install_ui.cc b/chrome/browser/extensions/extension_install_ui.cc
index 00c6a51..1c21536 100644
--- a/chrome/browser/extensions/extension_install_ui.cc
+++ b/chrome/browser/extensions/extension_install_ui.cc
@@ -43,20 +43,17 @@
 // static
 const int ExtensionInstallUI::kTitleIds[NUM_PROMPT_TYPES] = {
   IDS_EXTENSION_INSTALL_PROMPT_TITLE,
-  IDS_EXTENSION_UNINSTALL_PROMPT_TITLE,
-  IDS_EXTENSION_ENABLE_INCOGNITO_PROMPT_TITLE
+  IDS_EXTENSION_UNINSTALL_PROMPT_TITLE
 };
 // static
 const int ExtensionInstallUI::kHeadingIds[NUM_PROMPT_TYPES] = {
   IDS_EXTENSION_INSTALL_PROMPT_HEADING,
-  IDS_EXTENSION_UNINSTALL_PROMPT_HEADING,
-  IDS_EXTENSION_ENABLE_INCOGNITO_PROMPT_HEADING
+  IDS_EXTENSION_UNINSTALL_PROMPT_HEADING
 };
 // static
 const int ExtensionInstallUI::kButtonIds[NUM_PROMPT_TYPES] = {
   IDS_EXTENSION_PROMPT_INSTALL_BUTTON,
-  IDS_EXTENSION_PROMPT_UNINSTALL_BUTTON,
-  IDS_EXTENSION_PROMPT_ENABLE_INCOGNITO_BUTTON
+  IDS_EXTENSION_PROMPT_UNINSTALL_BUTTON
 };
 
 namespace {
@@ -182,15 +179,6 @@ void ExtensionInstallUI::ConfirmUninstall(Delegate* delegate,
   ShowConfirmation(UNINSTALL_PROMPT);
 }
 
-void ExtensionInstallUI::ConfirmEnableIncognito(Delegate* delegate,
-                                                Extension* extension) {
-  DCHECK(ui_loop_ == MessageLoop::current());
-  extension_ = extension;
-  delegate_ = delegate;
-
-  ShowConfirmation(ENABLE_INCOGNITO_PROMPT);
-}
-
 void ExtensionInstallUI::OnInstallSuccess(Extension* extension) {
   if (extension->IsTheme()) {
     ShowThemeInfoBar(previous_theme_id_, previous_use_system_theme_,
@@ -277,14 +265,6 @@ void ExtensionInstallUI::OnImageLoaded(
           message, UNINSTALL_PROMPT);
       break;
     }
-    case ENABLE_INCOGNITO_PROMPT: {
-      string16 message =
-          l10n_util::GetStringFUTF16(IDS_EXTENSION_PROMPT_WARNING_INCOGNITO,
-          l10n_util::GetStringUTF16(IDS_PRODUCT_NAME));
-      ShowExtensionInstallUIPromptImpl(profile_, delegate_, extension_, &icon_,
-          message, ENABLE_INCOGNITO_PROMPT);
-      break;
-    }
     default:
       NOTREACHED() << "Unknown message";
       break;
diff --git a/chrome/browser/extensions/extension_install_ui.h b/chrome/browser/extensions/extension_install_ui.h
index 1fb4b87..e99a7ac 100755
--- a/chrome/browser/extensions/extension_install_ui.h
+++ b/chrome/browser/extensions/extension_install_ui.h
@@ -30,7 +30,6 @@ class ExtensionInstallUI : public ImageLoadingTracker::Observer {
   enum PromptType {
     INSTALL_PROMPT = 0,
     UNINSTALL_PROMPT,
-    ENABLE_INCOGNITO_PROMPT,
     NUM_PROMPT_TYPES
   };
 
@@ -68,13 +67,6 @@ class ExtensionInstallUI : public ImageLoadingTracker::Observer {
   // on |delegate|.
   virtual void ConfirmUninstall(Delegate* delegate, Extension* extension);
 
-  // This is called by the extensions management page to verify whether the
-  // uninstallation should proceed. This is declared virtual for testing.
-  //
-  // We *MUST* eventually call either Proceed() or Abort()
-  // on |delegate|.
-  virtual void ConfirmEnableIncognito(Delegate* delegate, Extension* extension);
-
   // Installation was successful. This is declared virtual for testing.
   virtual void OnInstallSuccess(Extension* extension);
 
diff --git a/chrome/browser/extensions/extensions_ui.cc b/chrome/browser/extensions/extensions_ui.cc
index fc2850c..75b54e6 100644
--- a/chrome/browser/extensions/extensions_ui.cc
+++ b/chrome/browser/extensions/extensions_ui.cc
@@ -125,6 +125,8 @@ void ExtensionsUIHTMLSource::StartDataRequest(const std::string& path,
       l10n_util::GetString(IDS_EXTENSIONS_ENABLE));
   localized_strings.SetString(L"enableIncognito",
       l10n_util::GetString(IDS_EXTENSIONS_ENABLE_INCOGNITO));
+  localized_strings.SetString(L"incognitoWarning",
+      l10n_util::GetString(IDS_EXTENSIONS_INCOGNITO_WARNING));
   localized_strings.SetString(L"reload",
       l10n_util::GetString(IDS_EXTENSIONS_RELOAD));
   localized_strings.SetString(L"uninstall",
@@ -262,7 +264,7 @@ void ExtensionsDOMHandler::IconLoader::ReportResultOnUIThread(
 ///////////////////////////////////////////////////////////////////////////////
 
 ExtensionsDOMHandler::ExtensionsDOMHandler(ExtensionsService* extension_service)
-    : extensions_service_(extension_service) {
+    : extensions_service_(extension_service), ignore_notifications_(false) {
 }
 
 void ExtensionsDOMHandler::RegisterMessages() {
@@ -451,18 +453,20 @@ void ExtensionsDOMHandler::HandleEnableIncognitoMessage(const Value* value) {
                                                                true);
   DCHECK(extension);
 
-  if (enable_str == "true") {
-    if (!extension_id_prompting_.empty())
-      return;  // only one prompt at a time
-
-    // Prompt the user first.
-    ui_prompt_type_ = ExtensionInstallUI::ENABLE_INCOGNITO_PROMPT;
-    extension_id_prompting_ = extension_id;
-
-    GetExtensionInstallUI()->ConfirmEnableIncognito(this, extension);
-  } else {
-    extensions_service_->SetIsIncognitoEnabled(extension, false);
-  }
+  // Flipping the incognito bit will generate unload/load notifications for the
+  // extension, but we don't want to reload the page, because a) we've already
+  // updated the UI to reflect the change, and b) we want the yellow warning
+  // text to stay until the user has left the page.
+  //
+  // TODO(aa): This creates crapiness in some cases. For example, in a main
+  // window, when toggling this, the browser action will flicker because it gets
+  // unloaded, then reloaded. It would be better to have a dedicated
+  // notification for this case.
+  //
+  // Bug: http://crbug.com/41384
+  ignore_notifications_ = true;
+  extensions_service_->SetIsIncognitoEnabled(extension, enable_str == "true");
+  ignore_notifications_ = false;
 }
 
 void ExtensionsDOMHandler::HandleUninstallMessage(const Value* value) {
@@ -480,7 +484,6 @@ void ExtensionsDOMHandler::HandleUninstallMessage(const Value* value) {
   if (!extension_id_prompting_.empty())
     return;  // only one prompt at a time
 
-  ui_prompt_type_ = ExtensionInstallUI::UNINSTALL_PROMPT;
   extension_id_prompting_ = extension_id;
 
   GetExtensionInstallUI()->ConfirmUninstall(this, extension);
@@ -491,6 +494,8 @@ void ExtensionsDOMHandler::InstallUIProceed(bool create_app_shortcut) {
   // result in it telling us to create a shortcut.
   DCHECK(!create_app_shortcut);
 
+  DCHECK(!extension_id_prompting_.empty());
+
   // The extension can be uninstalled in another window while the UI was
   // showing. Do nothing in that case.
   Extension* extension =
@@ -498,19 +503,8 @@ void ExtensionsDOMHandler::InstallUIProceed(bool create_app_shortcut) {
   if (!extension)
     return;
 
-  switch (ui_prompt_type_) {
-    case ExtensionInstallUI::UNINSTALL_PROMPT:
-      extensions_service_->UninstallExtension(extension_id_prompting_,
-                                              false /* external_uninstall */);
-      break;
-    case ExtensionInstallUI::ENABLE_INCOGNITO_PROMPT:
-      extensions_service_->SetIsIncognitoEnabled(extension, true);
-      break;
-    default:
-      NOTREACHED();
-      break;
-  }
-
+  extensions_service_->UninstallExtension(extension_id_prompting_,
+                                          false /* external_uninstall */);
   extension_id_prompting_ = "";
 }
 
@@ -683,7 +677,7 @@ void ExtensionsDOMHandler::Observe(NotificationType type,
     case NotificationType::EXTENSION_UPDATE_DISABLED:
     case NotificationType::EXTENSION_FUNCTION_DISPATCHER_CREATED:
     case NotificationType::EXTENSION_FUNCTION_DISPATCHER_DESTROYED:
-      if (dom_ui_->tab_contents())
+      if (!ignore_notifications_ && dom_ui_->tab_contents())
         HandleRequestExtensionsData(NULL);
       break;
 
diff --git a/chrome/browser/extensions/extensions_ui.h b/chrome/browser/extensions/extensions_ui.h
index 8a96c54..b7c05ce 100644
--- a/chrome/browser/extensions/extensions_ui.h
+++ b/chrome/browser/extensions/extensions_ui.h
@@ -221,16 +221,17 @@ class ExtensionsDOMHandler
   // incognito mode.
   scoped_ptr<ExtensionInstallUI> install_ui_;
 
+  // The id of the extension we are prompting the user about.
+  std::string extension_id_prompting_;
+
   // We monitor changes to the extension system so that we can reload when
   // necessary.
   NotificationRegistrar registrar_;
 
-  // The id of the extension we are prompting the user about.
-  std::string extension_id_prompting_;
-
-  // The type of prompt that is open. Only ever uninstall or enable-incognito.
-  // Invalid if no prompt is open.
-  ExtensionInstallUI::PromptType ui_prompt_type_;
+  // If true, we will ignore notifications in ::Observe(). This is needed
+  // to prevent reloading the page when we were the cause of the
+  // notification.
+  bool ignore_notifications_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionsDOMHandler);
 };
diff --git a/chrome/browser/resources/extensions_ui.html b/chrome/browser/resources/extensions_ui.html
index 34de1af..42691d2 100644
--- a/chrome/browser/resources/extensions_ui.html
+++ b/chrome/browser/resources/extensions_ui.html
@@ -323,6 +323,19 @@ html[dir=rtl] #dialogContentFooter {
 .inspectPopupNote {
   color: grey;
 }
+
+.incognitoWarning {
+  margin: 0.75em 0;
+  display: none;
+  opacity: 0;
+  -webkit-transition: opacity .2s ease-out;
+}
+
+.incognitoWarning .yellow {
+  background:#fff299;
+  padding:2px 5px;
+  border-radius:3px;
+}
 </style>
 <script>
 /**
@@ -543,15 +556,27 @@ function handleEnableExtension(node, enable) {
 /**
  * Handles the 'enableIncognito' checkbox getting changed.
  */
-function handleToggleExtensionIncognito(node, toggleCheckbox) {
-  var enable = toggleCheckbox ? !node.checked : node.checked;
+function handleToggleExtensionIncognito(node) {
+  var warning = node;
 
-  // This is kinda cheesy, but if we're enabling, we have to wait for the
-  // prompt, so don't show as enabled until the user says OK.
-  node.checked = false;
+  while (warning.className != "extension")
+    warning = warning.parentNode;
 
-  chrome.send('enableIncognito', [node.extensionId, String(enable)]);
-  requestExtensionsData();
+  warning = warning.getElementsByClassName("incognitoWarning")[0];
+  if (!node.checked) {
+    warning.style.display = "none";
+    warning.style.opacity = "0";
+  } else {
+    warning.style.display = "block";
+
+    // Must set the opacity later. Otherwise, the fact that the display is
+    // changing causes the animation to not happen.
+    window.setTimeout(function() {
+      warning.style.opacity = "1";
+    }, 0);
+  }
+
+  chrome.send('enableIncognito', [node.extensionId, String(node.checked)]);
 }
 
 /**
@@ -851,18 +876,20 @@ document.addEventListener('DOMContentLoaded', requestExtensionsData);
                   href="javascript:void 0;"
                   i18n-content="options"
                   >OPTIONS</a>
+                <label
+                  jsdisplay="enabled"
+                  onclick="handleToggleExtensionIncognito(this.getElementsByTagName('input')[0])">
                 <input type="checkbox"
                   jsvalues=".extensionId:id;.enabled:enabled"
                   jsdisplay="enabled"
                   jseval="this.checked = enabledIncognito"
-                  onchange="handleToggleExtensionIncognito(this, false)">
-                <label
-                  jsdisplay="enabled"
-                  onclick="handleToggleExtensionIncognito(this.previousSibling.previousSibling, true)"
-                  i18n-content="enableIncognito">
-                  ALLOW THIS EXTENSION TO RUN IN INCOGNITO</label>
+                  onchange="handleToggleExtensionIncognito(this)">
+                <span i18n-content="enableIncognito">ALLOW THIS EXTENSION TO RUN IN INCOGNITO</span></label>
               </span>
             </div>
+            <div class="incognitoWarning">
+              <span class="yellow" i18n-values=".innerHTML:incognitoWarning">WARNING - CHROME CANNOT PREVENT THIS EXTENSION FROM RECORDING YOUR BROWSING HISTORY</span>
+            </div>
           </td>
           </tr>
           </table>