Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(118)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 848833002: 1. Adds null check when keymap is failed to load. 2. Moves xkb_rule_names construction into worker … (Closed)

Created:
5 years, 11 months ago by FengYuan
Modified:
5 years, 11 months ago
CC:
chromium-reviews, kalyank, tdresser+watch_chromium.org, jdduke+watch_chromium.org, ozone-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

1. Adds null check when keymap is failed to load. 2. Moves xkb_rule_names construction into worker thread to fix crash problem. BUG=447415, 444121 Committed: https://crrev.com/a15f89531db60f3397c04adf1a5fdc08f0f058cc Cr-Commit-Position: refs/heads/master@{#311421}

Patch Set 1 #

Total comments: 2

Patch Set 2 : #

Total comments: 2

Patch Set 3 : #

Patch Set 4 : #

Total comments: 2

Patch Set 5 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+46 lines, -43 lines) Patch
M ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.h View 1 chunk +0 lines, -4 lines 0 comments Download
M ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc View 1 2 3 4 4 chunks +40 lines, -35 lines 0 comments Download
M ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine_unittest.cc View 1 2 3 1 chunk +6 lines, -4 lines 0 comments Download

Messages

Total messages: 16 (3 generated)
FengYuan
5 years, 11 months ago (2015-01-13 06:13:15 UTC) #2
Shu Chen
https://codereview.chromium.org/848833002/diff/1/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc File ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc (right): https://codereview.chromium.org/848833002/diff/1/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc#newcode647 ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc:647: .layout = layout_id.c_str(), this will cause .layout carry wild ...
5 years, 11 months ago (2015-01-13 12:49:14 UTC) #3
FengYuan
https://codereview.chromium.org/848833002/diff/1/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc File ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc (right): https://codereview.chromium.org/848833002/diff/1/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc#newcode647 ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc:647: .layout = layout_id.c_str(), On 2015/01/13 12:49:14, Shu Chen wrote: ...
5 years, 11 months ago (2015-01-13 14:10:26 UTC) #4
kpschoedel
https://codereview.chromium.org/848833002/diff/20001/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc File ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc (right): https://codereview.chromium.org/848833002/diff/20001/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc#newcode646 ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc:646: char* layout_variant_buffer = new char[layout_variant.size() + 1]; These leak, ...
5 years, 11 months ago (2015-01-13 16:55:09 UTC) #5
FengYuan
PTAL. https://codereview.chromium.org/848833002/diff/20001/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc File ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc (right): https://codereview.chromium.org/848833002/diff/20001/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc#newcode646 ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc:646: char* layout_variant_buffer = new char[layout_variant.size() + 1]; On ...
5 years, 11 months ago (2015-01-14 01:56:40 UTC) #6
kpschoedel
lgtm https://codereview.chromium.org/848833002/diff/60001/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc File ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc (right): https://codereview.chromium.org/848833002/diff/60001/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc#newcode671 ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc:671: LOG(ERROR) << "Keymap file fail to load: " ...
5 years, 11 months ago (2015-01-14 05:01:42 UTC) #7
FengYuan
https://codereview.chromium.org/848833002/diff/60001/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc File ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc (right): https://codereview.chromium.org/848833002/diff/60001/ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc#newcode671 ui/events/ozone/layout/xkb/xkb_keyboard_layout_engine.cc:671: LOG(ERROR) << "Keymap file fail to load: " << ...
5 years, 11 months ago (2015-01-14 05:05:40 UTC) #8
kpschoedel
> > Nit: "failed" > > Done. Thanks; ++LGTM.
5 years, 11 months ago (2015-01-14 05:07:02 UTC) #9
FengYuan
Thanks for your review.
5 years, 11 months ago (2015-01-14 05:12:28 UTC) #11
rjkroege
On 2015/01/14 05:12:28, FengYuan wrote: > Thanks for your review. lgtm
5 years, 11 months ago (2015-01-14 05:13:25 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/848833002/80001
5 years, 11 months ago (2015-01-14 05:48:58 UTC) #14
commit-bot: I haz the power
Committed patchset #5 (id:80001)
5 years, 11 months ago (2015-01-14 06:48:13 UTC) #15
commit-bot: I haz the power
5 years, 11 months ago (2015-01-14 06:49:01 UTC) #16
Message was sent while issue was closed. 
Patchset 5 (id:??) landed as
https://crrev.com/a15f89531db60f3397c04adf1a5fdc08f0f058cc
Cr-Commit-Position: refs/heads/master@{#311421}

Powered by Google App Engine
This is Rietveld 408576698