|
|
Created:
4 years ago by rune Modified:
4 years ago Reviewers:
fs, foolip CC:
blink-reviews, chromium-reviews Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFixed flaky fullscreen video test.
The media controls are (at least sometimes) painted twice. The second
time after figuring out how many buttons fit into the available width.
At least one of the fullscreen tests were flaky because the first paint
was dumped instead of the second one. Adding a layoutAndPaintAsyncThen
in the full-screen-test.js framework seems to fix the issue.
Although the issue was filed after landing changes for issue 567021,
the flakiness is also seen locally without those changes running the
test with --repeat-each=30 in debug.
R=fs@opera.com
TEST=virtual/android/fullscreen/full-screen-stacking-context.html
BUG=675055
Committed: https://crrev.com/9d2592279f64b3a80329edef372adf6283349a0b
Cr-Commit-Position: refs/heads/master@{#439483}
Patch Set 1 #Patch Set 2 : Return early from endTest() if already called. #
Created: 4 years ago
Messages
Total messages: 18 (10 generated)
The CQ bit was checked by rune@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
ptal
fs@opera.com changed reviewers: + foolip@chromium.org
LGTM, but adding someone working on fullscreen for a FYI or opinions
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
I had to add a return from endTest() as layoutAndPaintAsyncThen() will trigger a DCHECK if called a second time before the async callback is invoked.
The CQ bit was checked by rune@opera.com
The patchset sent to the CQ was uploaded after l-g-t-m from fs@opera.com, foolip@chromium.org Link to the patchset: https://codereview.chromium.org/2586243002/#ps20001 (title: "Return early from endTest() if already called.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2016/12/19 at 14:45:00, rune wrote: > I had to add a return from endTest() as layoutAndPaintAsyncThen() will trigger a DCHECK if called a second time before the async callback is invoked. Still LGTM (but hrm...)
CQ is committing da patch. Bot data: {"patchset_id": 20001, "attempt_start_ts": 1482158884852420, "parent_rev": "e1e264fae227a604c05dcb245e14ec5674dd6879", "commit_rev": "712ab2951db7a2617ac63283084f58a337864cac"}
Message was sent while issue was closed.
Description was changed from ========== Fixed flaky fullscreen video test. The media controls are (at least sometimes) painted twice. The second time after figuring out how many buttons fit into the available width. At least one of the fullscreen tests were flaky because the first paint was dumped instead of the second one. Adding a layoutAndPaintAsyncThen in the full-screen-test.js framework seems to fix the issue. Although the issue was filed after landing changes for issue 567021, the flakiness is also seen locally without those changes running the test with --repeat-each=30 in debug. R=fs@opera.com TEST=virtual/android/fullscreen/full-screen-stacking-context.html BUG=675055 ========== to ========== Fixed flaky fullscreen video test. The media controls are (at least sometimes) painted twice. The second time after figuring out how many buttons fit into the available width. At least one of the fullscreen tests were flaky because the first paint was dumped instead of the second one. Adding a layoutAndPaintAsyncThen in the full-screen-test.js framework seems to fix the issue. Although the issue was filed after landing changes for issue 567021, the flakiness is also seen locally without those changes running the test with --repeat-each=30 in debug. R=fs@opera.com TEST=virtual/android/fullscreen/full-screen-stacking-context.html BUG=675055 Review-Url: https://codereview.chromium.org/2586243002 Committed: https://chromium.googlesource.com/chromium/src/+/712ab2951db7a2617ac63283084f... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/712ab2951db7a2617ac63283084f...
Message was sent while issue was closed.
Description was changed from ========== Fixed flaky fullscreen video test. The media controls are (at least sometimes) painted twice. The second time after figuring out how many buttons fit into the available width. At least one of the fullscreen tests were flaky because the first paint was dumped instead of the second one. Adding a layoutAndPaintAsyncThen in the full-screen-test.js framework seems to fix the issue. Although the issue was filed after landing changes for issue 567021, the flakiness is also seen locally without those changes running the test with --repeat-each=30 in debug. R=fs@opera.com TEST=virtual/android/fullscreen/full-screen-stacking-context.html BUG=675055 Review-Url: https://codereview.chromium.org/2586243002 Committed: https://chromium.googlesource.com/chromium/src/+/712ab2951db7a2617ac63283084f... ========== to ========== Fixed flaky fullscreen video test. The media controls are (at least sometimes) painted twice. The second time after figuring out how many buttons fit into the available width. At least one of the fullscreen tests were flaky because the first paint was dumped instead of the second one. Adding a layoutAndPaintAsyncThen in the full-screen-test.js framework seems to fix the issue. Although the issue was filed after landing changes for issue 567021, the flakiness is also seen locally without those changes running the test with --repeat-each=30 in debug. R=fs@opera.com TEST=virtual/android/fullscreen/full-screen-stacking-context.html BUG=675055 Committed: https://crrev.com/9d2592279f64b3a80329edef372adf6283349a0b Cr-Commit-Position: refs/heads/master@{#439483} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/9d2592279f64b3a80329edef372adf6283349a0b Cr-Commit-Position: refs/heads/master@{#439483} |