[HBD] Update Old Options Strings for HBD

chrome/browser/ui/webui/options/content_settings_handler.cc

Index: chrome/browser/ui/webui/options/content_settings_handler.cc
diff --git a/chrome/browser/ui/webui/options/content_settings_handler.cc b/chrome/browser/ui/webui/options/content_settings_handler.cc
index 071a24e705a6fa114f303c38f559900c879306e5..ccb95a27806d3c49049aaef5cb72ecbf5b660cd2 100644
--- a/chrome/browser/ui/webui/options/content_settings_handler.cc
+++ b/chrome/browser/ui/webui/options/content_settings_handler.cc
@@ -39,6 +39,7 @@
 #include "chrome/browser/ui/webui/site_settings_helper.h"
 #include "chrome/browser/usb/usb_chooser_context.h"
 #include "chrome/browser/usb/usb_chooser_context_factory.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
 #include "chrome/common/features.h"
@@ -372,7 +373,6 @@ void ContentSettingsHandler::GetLocalizedValues(
     {"allowException", IDS_EXCEPTIONS_ALLOW_BUTTON},
     {"blockException", IDS_EXCEPTIONS_BLOCK_BUTTON},
     {"sessionException", IDS_EXCEPTIONS_SESSION_ONLY_BUTTON},
-    {"detectException", IDS_EXCEPTIONS_DETECT_IMPORTANT_CONTENT_BUTTON},
     {"askException", IDS_EXCEPTIONS_ASK_BUTTON},
     {"otrExceptionsExplanation", IDS_EXCEPTIONS_OTR_LABEL},
     {"addNewExceptionInstructions", IDS_EXCEPTIONS_ADD_NEW_INSTRUCTIONS},
@@ -412,7 +412,6 @@ void ContentSettingsHandler::GetLocalizedValues(
     {"pluginsHeader", IDS_PLUGIN_HEADER},
     {"pluginsAllow", IDS_PLUGIN_ALLOW_RADIO},
     {"pluginsBlock", IDS_PLUGIN_BLOCK_RADIO},
-    {"pluginsDetectImportantContent", IDS_PLUGIN_DETECT_RECOMMENDED_RADIO},
     {"manageIndividualPlugins", IDS_PLUGIN_MANAGE_INDIVIDUAL},
     // Pop-ups filter.
     {"popupsTabLabel", IDS_POPUP_TAB_LABEL},
@@ -510,6 +509,23 @@ void ContentSettingsHandler::GetLocalizedValues(
 
   RegisterStrings(localized_strings, resources, arraysize(resources));
 
+  // TODO(tommycli): When the HTML5 By Default feature flag is on, we want to
+  // display strings that begin with "Ask...", even though the setting remains
+  // DETECT. Once this feature is finalized, then we migrate the setting to ASK.

[Dan Beam, 2016/09/01 06:15:15]

wat

[tommycli, 2016/09/01 17:59:33]

Acknowledged.

+  if (base::FeatureList::IsEnabled(features::kPreferHtmlOverPlugins)) {
+    static OptionsStringResource flash_strings[] = {
+        {"pluginsDetectImportantContent", IDS_PLUGIN_ASK_RECOMMENDED_RADIO},
+        {"detectException", IDS_EXCEPTIONS_ASK_BUTTON},
+    };
+    RegisterStrings(localized_strings, flash_strings, arraysize(flash_strings));
+  } else {
+    static OptionsStringResource flash_strings[] = {
+        {"pluginsDetectImportantContent", IDS_PLUGIN_DETECT_RECOMMENDED_RADIO},
+        {"detectException", IDS_EXCEPTIONS_DETECT_IMPORTANT_CONTENT_BUTTON},
+    };
+    RegisterStrings(localized_strings, flash_strings, arraysize(flash_strings));
+  }

[Dan Beam, 2016/09/01 06:15:15]

please tell me there's any other way to do this... that's less cray

[tommycli, 2016/09/01 17:59:33]

This is unfortunate. The story is:

ASK was originally Left Click to Play. Since we weren't ready to kill Left C2P
yet, and because DETECT is weaker than ASK, we created a new DETECT setting.
Legacy ASK users are mapped to BLOCK.

Now HBD is semantically closest to ASK, but in terms of security danger, it's
closest to DETECT, since we're exempting sites with any Site Engagement
currently.

Long story short: We aren't ready to start migrating peoples' content settings
yet, especially while it's a feature flag. Once HBD ships and it's done, we will
clean up the content settings.

[tommycli, 2016/09/01 18:04:25]

I could just also change IDS_PLUGIN_ASK_RECOMMEND_RADIO to
IDS_PLUGIN_HTML_BY_DEFAULT_RECOMMENDED_RADIO, if that would prevent confusion.

I just called it ASK because the string starts with "Ask first..." and
eventually we want to migrate the DETECT setting to ASK. (After the SEI
threshold is high. See schedule here:
https://docs.google.com/document/d/1iHhg7wxYTgBHsTzcfaZy36UJ33JmjRLro8TZR6lb0...)

[raymes, 2016/09/14 01:48:05]

I agree it's confusing but I can't really think of a better way. The only thing
I can think of that may help would be to have some functions in a util class
which abstract the indirection we're adding everywhere, e.g.:

GetPluginsDetectImportantContentString
GetDetectExceptionString

This wouldn't help in all the cases though, e.g. with some of the cases
https://codereview.chromium.org/2291323003. So I don't feel strongly about it. 

I don't know this code so well, so if you want an extra pair of eyes you may
want to add bauerb. 

[tommycli, 2016/09/14 20:55:16]

Okay sounds good, I'll add bauerb. I don't want to add any more abstractions
unless it's going to be used in multiple places. I think this is the best
(temporary) solution.

+
   PrefService* prefs = Profile::FromWebUI(web_ui())->GetPrefs();
   const base::Value* default_pref = prefs->GetDefaultPrefValue(
       content_settings::WebsiteSettingsRegistry::GetInstance()