1. Adds null check when keymap is failed to load. 2. Moves xkb_rule_names construction into worker …

ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc

Index: ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc
diff --git a/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc b/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc
index 49f29a118aa0f21f90a28e786fb984a6352f0799..3526670b7da5d36cfc78effb2042927c56b7ba1a 100644
--- a/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc
+++ b/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc
@@ -625,20 +625,48 @@ const PrintableSimpleEntry kSimpleMap[] = {
     {0x0259, VKEY_OEM_3},      // schwa
 };
 
+scoped_ptr<xkb_rule_names> GetXkbRuleNames(const std::string& layout_name) {
+  size_t dash_index = layout_name.find('-');
+  size_t parentheses_index = layout_name.find('(');
+  std::string layout_id = layout_name;
+  std::string layout_variant = "";
+  if (parentheses_index != std::string::npos) {
+    layout_id = layout_name.substr(0, parentheses_index);
+    size_t close_index = layout_name.find(')', parentheses_index);
+    if (close_index == std::string::npos)
+      close_index = layout_name.size();
+    layout_variant = layout_name.substr(parentheses_index + 1,
+                                        close_index - parentheses_index - 1);
+  } else if (dash_index != std::string::npos) {
+    layout_id = layout_name.substr(0, dash_index);
+    layout_variant = layout_name.substr(dash_index + 1);
+  }
+  return make_scoped_ptr<xkb_rule_names>(
+      new xkb_rule_names{.rules = NULL,
+                         .model = "pc101",
+                         .layout = layout_id.c_str(),

[Shu Chen, 2015/01/13 12:49:14]

this will cause .layout carry wild pointer, same for the .variant below.

[FengYuan, 2015/01/13 14:10:25]

Done.

+                         .variant = layout_variant.c_str(),
+                         .options = ""});
+}
+
 void LoadKeymap(const std::string& layout_name,
-                scoped_ptr<xkb_rule_names> names,
                 scoped_refptr<base::SingleThreadTaskRunner> reply_runner,
                 const LoadKeymapCallback& reply_callback) {
+  scoped_ptr<xkb_rule_names> names = GetXkbRuleNames(layout_name);
   scoped_ptr<xkb_context, XkbContextDeleter> context;
   context.reset(xkb_context_new(XKB_CONTEXT_NO_DEFAULT_INCLUDES));
   xkb_context_include_path_append(context.get(), "/usr/share/X11/xkb");
   scoped_ptr<xkb_keymap, XkbKeymapDeleter> keymap;
   keymap.reset(xkb_keymap_new_from_names(context.get(), names.get(),
                                          XKB_KEYMAP_COMPILE_NO_FLAGS));
-  char* keymap_str =
-      xkb_keymap_get_as_string(keymap.get(), XKB_KEYMAP_FORMAT_TEXT_V1);
-  reply_runner->PostTask(FROM_HERE, base::Bind(reply_callback, layout_name,
-                                               base::Owned(keymap_str)));
+  if (keymap) {
+    char* keymap_str =
+        xkb_keymap_get_as_string(keymap.get(), XKB_KEYMAP_FORMAT_TEXT_V1);
+    reply_runner->PostTask(FROM_HERE, base::Bind(reply_callback, layout_name,
+                                                 base::Owned(keymap_str)));
+  } else {
+    LOG(ERROR) << "Keymap file fail to load: " << layout_name;
+  }
 }
 
 }  // anonymous namespace
@@ -686,11 +714,10 @@ bool XkbKeyboardLayoutEngine::SetCurrentLayoutByName(
   }
   LoadKeymapCallback reply_callback = base::Bind(
       &XkbKeyboardLayoutEngine::OnKeymapLoaded, weak_ptr_factory_.GetWeakPtr());
-  scoped_ptr<xkb_rule_names> names = GetXkbRuleNames(layout_name);
   base::WorkerPool::PostTask(
       FROM_HERE,
-      base::Bind(&LoadKeymap, layout_name, base::Passed(&names),
-                 base::ThreadTaskRunnerHandle::Get(), reply_callback),
+      base::Bind(&LoadKeymap, layout_name, base::ThreadTaskRunnerHandle::Get(),
+                 reply_callback),
       true);
 #else
   xkb_keymap* keymap = xkb_map_new_from_string(
@@ -703,31 +730,6 @@ bool XkbKeyboardLayoutEngine::SetCurrentLayoutByName(
   return true;
 }
 
-scoped_ptr<xkb_rule_names> XkbKeyboardLayoutEngine::GetXkbRuleNames(
-    const std::string& layout_name) {
-  size_t dash_index = layout_name.find('-');
-  size_t parentheses_index = layout_name.find('(');
-  std::string layout_id = layout_name;
-  std::string layout_variant = "";
-  if (parentheses_index != std::string::npos) {
-    layout_id = layout_name.substr(0, parentheses_index);
-    size_t close_index = layout_name.find(')', parentheses_index);
-    if (close_index == std::string::npos)
-      close_index = layout_name.size();
-    layout_variant = layout_name.substr(parentheses_index + 1,
-                                        close_index - parentheses_index - 1);
-  } else if (dash_index != std::string::npos) {
-    layout_id = layout_name.substr(0, dash_index);
-    layout_variant = layout_name.substr(dash_index + 1);
-  }
-  return make_scoped_ptr<xkb_rule_names>(
-      new xkb_rule_names{.rules = NULL,
-                         .model = "pc101",
-                         .layout = layout_id.c_str(),
-                         .variant = layout_variant.c_str(),
-                         .options = ""});
-}
-
 void XkbKeyboardLayoutEngine::OnKeymapLoaded(const std::string& layout_name,
                                              const char* keymap_str) {
   if (keymap_str) {