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

Issue 1295773002: Make testDone unavailable for chromevox tests. (Closed)

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

Description

Make testDone unavailable for chromevox tests. the testDone function is easy to use incorrectly and is from now on banned from chromevox tests. This cl removes all uses and adjsuts some tests accordingly. As a result, some chromevox next tests are disabled (because they never ran properly and are actually broken). This also fixes a race in the code that determines if chromevox classic should be enabled in a content script or not. This affected tests since they load multiple tabs simultaneously, but could also happen in actual usage since the time window for the race to trigger is on the order of seconds. BUG=520938, 520940, 520946 Committed: https://crrev.com/685ebec05c262ceff0f016aae4c8409ddeb7d515 Cr-Commit-Position: refs/heads/master@{#343641}

Patch Set 1 #

Patch Set 2 : Remove redundant bind calls. #

Patch Set 3 : n #

Unified diffs Side-by-side diffs Delta from patch set Stats (+391 lines, -35 lines) Patch
M chrome/browser/resources/chromeos/chromevox/chromevox_tests.gypi View 1 2 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/browser/resources/chromeos/chromevox/cvox2/background/background_test.extjs View 1 2 7 chunks +34 lines, -35 lines 0 comments Download
A chrome/browser/resources/chromeos/chromevox/testing/mock_feedback.js View 1 2 1 chunk +233 lines, -0 lines 0 comments Download
A chrome/browser/resources/chromeos/chromevox/testing/mock_feedback_test.unitjs View 1 2 1 chunk +122 lines, -0 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 7 (2 generated)
Peter Lundblad
5 years, 4 months ago (2015-08-14 13:21:18 UTC) #2
dmazzoni
lgtm
5 years, 4 months ago (2015-08-14 15:45:04 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1295773002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1295773002/20001
5 years, 4 months ago (2015-08-17 07:46:18 UTC) #5
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 4 months ago (2015-08-17 08:12:22 UTC) #6
commit-bot: I haz the power
5 years, 4 months ago (2015-08-17 08:12:57 UTC) #7
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/685ebec05c262ceff0f016aae4c8409ddeb7d515
Cr-Commit-Position: refs/heads/master@{#343641}

Powered by Google App Engine
This is Rietveld 408576698