separate about page into its own page (included in chrome://chrome)

chrome/browser/ui/webui/about_page/about_page_handler.cc

Index: chrome/browser/ui/webui/about_page/about_page_handler.cc
diff --git a/chrome/browser/ui/webui/options2/chromeos/about_page_handler2.cc b/chrome/browser/ui/webui/about_page/about_page_handler.cc
similarity index 96%
rename from chrome/browser/ui/webui/options2/chromeos/about_page_handler2.cc
rename to chrome/browser/ui/webui/about_page/about_page_handler.cc
index de472c81c17fd8c89c43e59535a41bbb0dfcd108..685642ec43c60d993ad0fe4f03ed9e1abaa64ffe 100644
--- a/chrome/browser/ui/webui/options2/chromeos/about_page_handler2.cc
+++ b/chrome/browser/ui/webui/about_page/about_page_handler.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/ui/webui/options2/chromeos/about_page_handler2.h"
+#include "chrome/browser/ui/webui/about_page/about_page_handler.h"
 
 #include <vector>
 
@@ -24,6 +24,7 @@
 #include "chrome/browser/google/google_util.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/url_constants.h"
+#include "content/browser/webui/web_ui.h"
 #include "content/public/common/content_client.h"
 #include "googleurl/src/gurl.h"
 #include "grit/chromium_strings.h"
@@ -59,8 +60,11 @@ std::string StringSubRange(const std::string& text, size_t start,
 
 }  // namespace
 
-namespace chromeos {
-namespace options2 {
+using chromeos::DBusThreadManager;
+using chromeos::UpdateEngineClient;
+using chromeos::UserManager;
+using chromeos::VersionLoader;
+using chromeos::WizardController;
 
 class AboutPageHandler::UpdateObserver
     : public UpdateEngineClient::Observer {
@@ -84,8 +88,8 @@ class AboutPageHandler::UpdateObserver
 AboutPageHandler::AboutPageHandler()
     : progress_(-1),
       sticky_(false),
-      started_(false)
-{}
+      started_(false) {
+}
 
 AboutPageHandler::~AboutPageHandler() {
   if (update_observer_.get()) {
@@ -97,7 +101,13 @@ AboutPageHandler::~AboutPageHandler() {
 void AboutPageHandler::GetLocalizedValues(DictionaryValue* localized_strings) {
   DCHECK(localized_strings);
 
-  static OptionsStringResource resources[] = {
+  struct L10nResources {
+    const char* name;
+    int ids;
+  };
+
+  static L10nResources resources[] = {
+    { "pageTitle", IDS_ABOUT_TAB_TITLE },

[Dan Beam, 2012/01/09 23:05:51]

Do you think it would be worth alpha sorting these to find one easier?

[Evan Stade, 2012/01/09 23:31:16]

no I prefer to keep the related ones together, e.g. stable near beta and os near
platform. Anyway it's not my call as I don't own this code.

[Dan Beam, 2012/01/09 23:53:14]

If it's in preference order or an order I'm not aware of, that's great, just
wanted that to have a significance (which it seems it does).

     { "firmware", IDS_ABOUT_PAGE_FIRMWARE },
     { "product", IDS_PRODUCT_OS_NAME },
     { "os", IDS_PRODUCT_OS_NAME },
@@ -120,8 +130,10 @@ void AboutPageHandler::GetLocalizedValues(DictionaryValue* localized_strings) {
     { "command_line", IDS_ABOUT_VERSION_COMMAND_LINE },
   };
 
-  RegisterStrings(localized_strings, resources, arraysize(resources));
-  RegisterTitle(localized_strings, "aboutPage", IDS_ABOUT_TAB_TITLE);
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(resources); ++i) {

[Dan Beam, 2012/01/09 23:05:51]

^ didn't know about this macro, cool!

[Evan Stade, 2012/01/09 23:31:16]

fyi, better to use arraysize() where possible

+    localized_strings->SetString(resources[i].name,
+                                 l10n_util::GetStringUTF16(resources[i].ids));

[Dan Beam, 2012/01/09 23:05:51]

Should we both DCHECK()ing that the key doesn't exist here?  Seems like it'd be
pretty easy to write over an existing key accidentally (which may be a bug or
feature, depending on how you look at it, but I think bug).

[Evan Stade, 2012/01/09 23:31:16]

I don't think so, it seems a little overly cautious. I can't recall ever
encountering a bug where a string was overwritten. This function is the only
place where any strings should be written to this dictionary so it's not like a
code change in another file could affect this list.

[Dan Beam, 2012/01/09 23:53:14]

But what happens when you start to merge dictionaries from various files by
passing in a partly populated DictionaryValue* via |localized_strings| (or
you're saying that Just Won't Happen™)?  Why not just return a new
DictionaryValue* to the caller, clear the DictionaryValue* at start, or DCHECK()
the keys being set (or key size at the beginning of the method).  Any of these
seem like good preventative solutions.

[Evan Stade, 2012/01/10 00:04:50]

that's indeed possible, if we wanted to fold about into another page. However to
actually be safe in that case we'd have to add DCHECKs for every SetString
(below as well) which would be onerous. I think if we're going to fold pages
together the person doing it should namespace the strings, i.e. put "about_" in
front of each one of them.
empty check added

+  }
 
   // browser version
 
@@ -416,6 +428,3 @@ void AboutPageHandler::UpdateSelectedChannel(UpdateObserver* observer,
         "AboutPage.updateSelectedOptionCallback", *channel_string);
   }
 }
-
-}  // namespace options2
-}  // namespace chromeos