|
|
DescriptionWait longer for actionmode menu in webview actionmode tests
BUG=711424
Review-Url: https://codereview.chromium.org/2819563003
Cr-Commit-Position: refs/heads/master@{#466827}
Committed: https://chromium.googlesource.com/chromium/src/+/4efeddc9c45b938f1c89b16f68938b3d9c7a31fc
Patch Set 1 #Patch Set 2 : Clarify the ActionBar idling resource logic #
Messages
Total messages: 19 (11 generated)
Description was changed from ========== Wait longer for actionmode menu in webview actionmode tests BUG=711424 ========== to ========== Wait longer for actionmode menu in webview actionmode tests BUG=711424 ==========
aluo@chromium.org changed reviewers: + mikecase@chromium.org, yolandyan@chromium.org
Ping
Could you explain how this code works? Not sure I completely understand what it is doing.
On 2017/04/18 20:43:33, mikecase wrote: > Could you explain how this code works? Not sure I completely understand what it > is doing. Espresso calls the IdlingResource.isIdleNow() method right away to determine if it needs to wait for any idling resources. Since usually the app does not yet have the actionbar visible when this happens, and the actionbar idling resource checks the visibility of the actionbar, the 1st call to isIdleNow() will normally return false. Then espresso waits 5 seconds to check again and so on. This allows enough time for the actionbar to appear before clicking on items inside of it. But in the case of doing a select all, followed by a copy, the actionbar is already visible at when select all is clicked, so when the copy action pre-check calls isIdleNow, it will return true right away. However there's a tiny delay for an updated actionbar to get recreated after select all is clicked, causing the actual click to happen on the old copy of the actionbar which does not trigger a valid copy. By returning false in isIdleNow initially, espresso is forced to wait at least 1 delay which gives the valid actionbar time to get re-created.
hmm, at very least we should add comment about why this is doing what it is. Since it doesnt seem intuitive. However, is there a simpler fix than what you descirbed. Yoland, do you have any ideas.
On 2017/04/20 00:35:38, mikecase wrote: > hmm, at very least we should add comment about why this is doing what it is. > Since it doesnt seem intuitive. However, is there a simpler fix than what you > descirbed. Yoland, do you have any ideas. done, I added comment to it.
lgtm
The CQ bit was checked by aluo@chromium.org
The CQ bit was unchecked by aluo@chromium.org
The CQ bit was checked by aluo@chromium.org 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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by aluo@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493077247931400, "parent_rev": "3b36efad6b17e10ae243a8668528aae310a312da", "commit_rev": "4efeddc9c45b938f1c89b16f68938b3d9c7a31fc"}
Message was sent while issue was closed.
Description was changed from ========== Wait longer for actionmode menu in webview actionmode tests BUG=711424 ========== to ========== Wait longer for actionmode menu in webview actionmode tests BUG=711424 Review-Url: https://codereview.chromium.org/2819563003 Cr-Commit-Position: refs/heads/master@{#466827} Committed: https://chromium.googlesource.com/chromium/src/+/4efeddc9c45b938f1c89b16f6893... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/4efeddc9c45b938f1c89b16f6893... |