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

 // Copyright 2014 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.
 
 #include "ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.h"
 
 #include <xkbcommon/xkbcommon-names.h>
 
 #include "base/bind.h"
 #include "base/location.h"
@@ skipping 607 matching lines @@
     {0x0163, VKEY_OEM_7},      // t cedilla
     {0x0165, VKEY_5},          // t caron
     {0x016F, VKEY_OEM_1},      // u ring above
     {0x0171, VKEY_OEM_5},      // u double acute
     {0x01A1, VKEY_OEM_6},      // o horn
     {0x01B0, VKEY_OEM_4},      // u horn
     {0x01B6, VKEY_OEM_6},      // z stroke
     {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
 
 XkbKeyCodeConverter::XkbKeyCodeConverter() {
 }
 
 XkbKeyCodeConverter::~XkbKeyCodeConverter() {
 }
 
@@ skipping 27 matching lines @@
 #if defined(OS_CHROMEOS)
   current_layout_name_ = layout_name;
   for (const auto& entry : xkb_keymaps_) {
     if (entry.layout_name == layout_name) {
       SetKeymap(entry.keymap);
       return true;
     }
   }
   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(
       xkb_context_.get(), layout_name.c_str(), XKB_KEYMAP_FORMAT_TEXT_V1,
       XKB_KEYMAP_COMPILE_NO_FLAGS);
   if (!keymap)
     return false;
   SetKeymap(keymap);
 #endif  // defined(OS_CHROMEOS)
   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) {
     xkb_keymap* keymap = xkb_map_new_from_string(xkb_context_.get(), keymap_str,
                                                  XKB_KEYMAP_FORMAT_TEXT_V1,
                                                  XKB_KEYMAP_COMPILE_NO_FLAGS);
     XkbKeymapEntry entry = {layout_name, keymap};
     xkb_keymaps_.push_back(entry);
     if (layout_name == current_layout_name_)
       SetKeymap(keymap);
@@ skipping 199 matching lines @@
   if (flags == base_flags)
     return base_character;
   xkb_keysym_t keysym;
   base::char16 character = 0;
   if (!XkbLookup(xkb_keycode, flags, &keysym, &character))
     character = kNone;
   return character;
 }
 
 }  // namespace ui