Add character encoding to Font Settings Extension API.

Add character encoding to Font Settings Extension API.

BUG=114148
TEST=browser_tests --gtest_filter=ExtensionApiTest.FontSettings


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=133405

[falken, 2012-04-20 10:17:17]

Matt: PTAL. By the way, I'm considering submitting
extension_font_settings_api.[cc,h] for C++ readability review. Do you think it
would be eligible?

Jungshik: FYI (I recall you wanted to move encoding out of font settings UI, but
I think it makes sense for the API, right?)

[Matt Perry, 2012-04-20 18:59:22]

LGTM

As for your readability review, go for it. I predict some nitpicks (like adding
DISALLOW_COPY_AND_ASSIGN to all the classes, which I'd really rather not do
here), but it's a good chunk of clean code IMO.

https://chromiumcodereview.appspot.com/10142011/diff/10001/chrome/browser/ext...
File chrome/browser/extensions/extension_font_settings_api.cc (right):

https://chromiumcodereview.appspot.com/10142011/diff/10001/chrome/browser/ext...
chrome/browser/extensions/extension_font_settings_api.cc:160:
std::make_pair(kOnDefaultCharacterSetChanged, kCharsetKey);
If there will be many more of these, I take back what I said about adding these
directly being less code :). Consider adding a helper function that adds the
pref to both the registrar and map.

https://chromiumcodereview.appspot.com/10142011/diff/10001/chrome/browser/ext...
File chrome/browser/extensions/extension_font_settings_api.h (right):

https://chromiumcodereview.appspot.com/10142011/diff/10001/chrome/browser/ext...
chrome/browser/extensions/extension_font_settings_api.h:26: typedef
std::map<std::string, std::pair<std::string, std::string> >
Can you typedef the std::pair to something so it's clearer what this map is?
Something like EventAndKeyPair is fine.

[falken, 2012-04-23 03:41:44]

Thanks!

http://codereview.chromium.org/10142011/diff/10001/chrome/browser/extensions/...
File chrome/browser/extensions/extension_font_settings_api.cc (right):

http://codereview.chromium.org/10142011/diff/10001/chrome/browser/extensions/...
chrome/browser/extensions/extension_font_settings_api.cc:160:
std::make_pair(kOnDefaultCharacterSetChanged, kCharsetKey);
On 2012/04/20 18:59:22, Matt Perry wrote:
> If there will be many more of these, I take back what I said about adding
these
> directly being less code :). Consider adding a helper function that adds the
> pref to both the registrar and map.

Done. I added a helper function.

http://codereview.chromium.org/10142011/diff/10001/chrome/browser/extensions/...
File chrome/browser/extensions/extension_font_settings_api.h (right):

http://codereview.chromium.org/10142011/diff/10001/chrome/browser/extensions/...
chrome/browser/extensions/extension_font_settings_api.h:26: typedef
std::map<std::string, std::pair<std::string, std::string> >
On 2012/04/20 18:59:22, Matt Perry wrote:
> Can you typedef the std::pair to something so it's clearer what this map is?
> Something like EventAndKeyPair is fine.

Done.

[commit-bot: I haz the power, 2012-04-23 03:43:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/falken@chromium.org/10142011/10002

[commit-bot: I haz the power, 2012-04-23 04:52:48]

Change committed as 133405