Surface button for loading stale cache copy on net error page.

chrome/common/localized_error.cc

Index: chrome/common/localized_error.cc
diff --git a/chrome/common/localized_error.cc b/chrome/common/localized_error.cc
index 2e0b80847f5fce79c75a78a6e7656643a1ca62c0..658c6dcb562620625466675746f9f9f2485d22fd 100644
--- a/chrome/common/localized_error.cc
+++ b/chrome/common/localized_error.cc
@@ -669,12 +669,17 @@ void LocalizedError::GetStrings(int error_code,
     }
   }
 
+  if (stale_copy_in_cache) {

[mmenke, 2014/03/26 15:06:14]

I suggest putting this below use_default_suggestions.  Suggesting loading from
Google's cache and a separate suggestion to load from the local cache just seems
very weird to me.

[Randy Smith (Not in Mondays), 2014/03/26 20:40:12]

This currently happens only in the link doctor case?  (That's what it looks like
to me from reading the code, but I want to check.)  I'm ok but a little
uncomfortable with this--the problem is that there's no reason in this
particular section of code why we'd make loading a stale copy dependent on
whether or not we're doing non-default suggestions; there isn't any reason why
the non-default suggestions would have anything to do with a cache.  But it's a
pretty minor point, and I agree with the goal.

[mmenke, 2014/03/27 14:28:28]

Yes, this is only in the Link Doctor case.  This is actually a "default
suggestion", as I see it (Though renaming the bool to suggest_load_from_cache
would change that opinion), but my real concern is just the two cache links, not
what is and is not "default".  If you want to just hide the button when Google
suggests we check it's cache, though that gets a bit hairy (And I've seen Link
Doctor lie about what's in Google's cache).

[Randy Smith (Not in Mondays), 2014/04/02 19:20:58]

Yeah, I thought about what it would take to do it the "right" way, and decided
against.  So yes, load stale cache is a default suggestion; I'm good with that
:-}.

+    base::DictionaryValue* stale_load_button = new base::DictionaryValue;
+    stale_load_button->SetString(
+        "msg", l10n_util::GetStringUTF16(IDS_ERRORPAGES_BUTTON_STALE));

[mmenke, 2014/03/26 15:06:14]

This subdirectory isn't needed.  Instead you can just set staleCopyInCache(Or
whatever) to be this string directly, and use the attibutes on the button:

jscontent="staleCopyInCache" jsdisplay="staleCopyInCache"

[Randy Smith (Not in Mondays), 2014/03/26 20:40:12]

Done (now that I understand what's going on :-}).

+    error_strings->Set("staleCopyInCache", stale_load_button);

[mmenke, 2014/03/26 15:06:14]

Getting the "IDS_ERRORPAGES_BUTTON_STALE" string, storing it in the
"stale_load_button" value, and then putting it in the "staleCopyInCache"
Javascript variable seems a little weird (Admittedly, this is the case with the
reload button as well).

I suggest renaming them to be more consistent.  Just to throw out one
suggestion:

IDS_ERRORPAGES_BUTTON_LOAD_STALE / stale_load_button / staleLoadButton

[Randy Smith (Not in Mondays), 2014/03/26 20:40:12]

Done.  Let me know if you'd like me to regularize the reload button.

[mmenke, 2014/03/27 14:28:28]

I think it'd be great if you went ahead and cleaned that up while you're here.

[Randy Smith (Not in Mondays), 2014/04/02 19:20:58]

I believe I've done this (just changing the templateData name from "reload" to
"reloadButton"); let me know if you think I've missed something.

+  }
+
   // If not using the default suggestions, nothing else to do.
   if (!use_default_suggestions)
     return;
 
-  error_strings->SetBoolean("staleCopyInCache", stale_copy_in_cache);
-
 #if defined(OS_CHROMEOS)
   error_strings->SetString(
       "diagnose", l10n_util::GetStringUTF16(IDS_ERRORPAGES_BUTTON_DIAGNOSE));