Implemented the rest of loading/parsing logic for extension i18n:...

chrome/browser/extensions/extension_message_handler.h

+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_MESSAGE_HANDLER_H_
+#define CHROME_BROWSER_EXTENSIONS_EXTENSION_MESSAGE_HANDLER_H_
+
+#include <string>
+
+#include "base/hash_tables.h"
+#include "base/values.h"
+
+// Parses both default and application dictionaries (passed in Init method),
+// and creates unified dictionary for lookup.
+// It simplifies message handling by substituting placeholders into messages.
+// It provides ReplaceVariablesInString method that can be used for both message
+// substitution in HTML files and for placeholder replacement within messages.
+class ExtensionMessageHandler {

[Aaron Boodman, 2009/08/31 21:42:55]

This class seems mis-named. It isn't really "handling" anything. How about
ExtensionMessageBundle?

+ public:
+  typedef base::hash_map<std::string, std::string> SSMap;

[Aaron Boodman, 2009/08/31 21:42:55]

Avoid abbreviations. Can you come up with a descriptive, but still relatively
short name for this type? MessageMap? SubstitutionMap?

+
+  // JSON keys of interest for messages file.
+  static const wchar_t* kContentKey;
+  static const wchar_t* kMessageKey;
+  static const wchar_t* kPlaceholdersKey;
+
+  // Begin/end markers for placeholders and messages
+  static const char* kPlaceholderBegin;
+  static const char* kPlaceholderEnd;
+  static const char* kMessageBegin;
+  static const char* kMessageEnd;
+
+  ExtensionMessageHandler();
+  ~ExtensionMessageHandler();
+
+  // Merges application and default dictionaries into one hash_map for later
+  // lookup.
+  // It takes key-values from |application_dictionary| and stores them in
+  // |dictionary_|.
+  // It then stores all unique messages in |default_dictionary| into
+  // |dictionary_|.
+  bool Init(const DictionaryValue* default_dictionary,

[Aaron Boodman, 2009/08/31 21:42:55]

In cases where initialization can fail, we tend to prefer a private constructor
with a static Create() method that can return NULL. That makes it so that you
can never end up with an invalid instance.

For an example of this pattern, see RSAPrivateKey.

+            const DictionaryValue* application_dictionary,
+            std::string* error);
+
+  // Get message with key |name|.
+  // Returns false if message is not in the dictionary.
+  bool GetMessage(const std::string& name, std::string* message) const;

[Aaron Boodman, 2009/08/31 21:42:55]

Nit: Can we just return std::string? It could be empty if the key doesn't exist.
If we ever find that we need to distinguish between empty string and
non-existent key, we could add a HasMessage() method.

+
+  // Dictionary size.
+  size_t GetDictionarySize() const {

[Aaron Boodman, 2009/08/31 21:42:55]

Is this just the number of messages? If so, maybe just size()? Or
num_messages()? The word "dictionary" seems out of place in the interface since
it isn't mentioned elsewhere.

+    return dictionary_.size();
+  }
+
+  // For a given |text| replaces all __MSG_message__ with values from
+  // dictionary_.
+  bool ReplaceMessagesInString(std::string* text) const;

[Aaron Boodman, 2009/08/31 21:42:55]

Nit: "InString" is unnecessary as that information is conveyed by the signature.

+
+  // Given list of variable names/values |variables|, and markers for their
+  // begin/end |var_begin|, |var_end|, replaces each occurance of variable
+  // placeholder with its value.
+  // I.e. replaces __MSG_name__ with value from |variables| that has name as a
+  // key.
+  // Returns false if for a valid message/placeholder name there is no matching
+  // replacement in |variables|. Sets |message| to that variable name.
+  // Public for easier unittesting.
+  bool ReplaceVariablesInString(const SSMap& variables,

[Aaron Boodman, 2009/08/31 21:42:55]

Looks like this should be static as it refers to no state in this class.

+                                const std::string& var_begin,

[Aaron Boodman, 2009/08/31 21:42:55]

Nit: maybe var_begin -> var_begin_delimiter? I thought this was a position from
its name alone.

+                                const std::string& var_end,
+                                std::string* message) const;
+
+  // Allow only ascii 0-9, a-z, A-Z, and _ in the variable name.
+  // Returns false if |name| is empty or if it has illegal characters.
+  // Public for easier unittesting.
+  bool IsValidName(const std::string& name) const;

[Aaron Boodman, 2009/08/31 21:42:55]

Looks like this should be static.

+
+ private:
+  // Helper methods that navigate JSON tree and return simplified message.
+  // They replace all $PLACEHOLDERS$ with their value, and return just key/value
+  // of the message.
+  bool GetMessageValue(const std::wstring& wkey,

[Aaron Boodman, 2009/08/31 21:42:55]

It is really odd to have a method take two keys in different encodings. Looking
at the implementation I can see why you did that, but you should probably
mention in the comments that the two keys should be the same.

+                       const std::string& key,
+                       const DictionaryValue* main_catalog,

[Aaron Boodman, 2009/08/31 21:42:55]

Chromium mainly uses const references for input params and non-const pointers
for output params.

+                       std::string* value,
+                       std::string* error) const;
+
+  // Get all placeholders for a given message from JSON subtree.
+  bool GetPlaceholders(const DictionaryValue* name_tree,
+                       const std::string name_key,
+                       SSMap* placeholders,
+                       std::string* error) const;
+
+  // For a given message, replaces all placeholders with their actual value.
+  // Returns false if replacement failed (see ReplaceVariablesInString).
+  bool ReplacePlaceholdersInMessage(const SSMap& placeholders,
+                                    std::string* message) const;
+
+  // Holds all messages for application locale.
+  SSMap dictionary_;
+};
+
+#endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_MESSAGE_HANDLER_H_