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

Issue 927313002: Fix and reenable liblouis chromevox tests. (Closed)

Created:
5 years, 10 months ago by Peter Lundblad
Modified:
5 years, 10 months ago
Reviewers:
David Tseng
CC:
chromium-reviews, oshima+watch_chromium.org, plundblad+watch_chromium.org, aboxhall+watch_chromium.org, nkostylev+watch_chromium.org, yuzo+watch_chromium.org, dmazzoni+watch_chromium.org, dtseng+watch_chromium.org, stevenjb+watch_chromium.org, arv+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@test_api
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Address flake in the liblouis wrapper. This fixes and reenables liblouis chromevox tests. Instead of relying on the 'load' event of the nacl component, just queue up requests to the component as soon as it is attached. This simplifies the state management in the wrapper. As a bonus, the tests are fixed to use the appropriate callback wrappers so that errors are immediately reported in failure cases instead of making the tests painfully block and time out. BUG=458910 Committed: https://crrev.com/eff7d2454cc5eff276b0a3e64da1219aead6b5d8 Cr-Commit-Position: refs/heads/master@{#316658}

Patch Set 1 #

Patch Set 2 : Simplify the state handling in the liblousi wrapper, add more tests. #

Patch Set 3 : Indent fix #

Patch Set 4 : Fix BrailleTranslatorManager tests that broke when fetchTables was moved into the event handler. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+124 lines, -119 lines) Patch
M chrome/browser/resources/chromeos/chromevox/braille/braille_translator_manager.js View 1 2 3 2 chunks +6 lines, -3 lines 0 comments Download
M chrome/browser/resources/chromeos/chromevox/braille/braille_translator_manager_test.extjs View 1 2 3 5 chunks +16 lines, -8 lines 0 comments Download
M chrome/browser/resources/chromeos/chromevox/braille/liblouis.js View 1 2 10 chunks +27 lines, -66 lines 0 comments Download
M chrome/browser/resources/chromeos/chromevox/braille/liblouis_test.extjs View 1 3 chunks +75 lines, -42 lines 0 comments Download

Messages

Total messages: 8 (3 generated)
Peter Lundblad
5 years, 10 months ago (2015-02-17 16:39:00 UTC) #2
David Tseng
lgtm
5 years, 10 months ago (2015-02-17 20:00:13 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/927313002/60001
5 years, 10 months ago (2015-02-17 21:14:39 UTC) #6
commit-bot: I haz the power
Committed patchset #4 (id:60001)
5 years, 10 months ago (2015-02-17 21:39:45 UTC) #7
commit-bot: I haz the power
5 years, 10 months ago (2015-02-17 21:40:44 UTC) #8
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/eff7d2454cc5eff276b0a3e64da1219aead6b5d8
Cr-Commit-Position: refs/heads/master@{#316658}

Powered by Google App Engine
This is Rietveld 408576698