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

Issue 2794883002: [headless] Fix race in TargetDomainCreateTwoContexts test. (Closed)

Created:
3 years, 8 months ago by Eric Seckler
Modified:
3 years, 8 months ago
Reviewers:
Sami
CC:
chromium-reviews, pfeldman, devtools-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[headless] Fix race in TargetDomainCreateTwoContexts test. There's a race between completion of the load of the url provided to Target.createTarget and the taking effect of Page.enable sent to the created targets. This patch resolves the race by separating target creation from page loading to initiate the page load after Page.enable. BUG=546953 Review-Url: https://codereview.chromium.org/2794883002 Cr-Commit-Position: refs/heads/master@{#461403} Committed: https://chromium.googlesource.com/chromium/src/+/bccb2a6db2d722ead168ce0244c4d9d2dc67adab

Patch Set 1 : . #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+67 lines, -17 lines) Patch
M headless/lib/headless_devtools_client_browsertest.cc View 6 chunks +67 lines, -17 lines 3 comments Download

Messages

Total messages: 15 (9 generated)
Eric Seckler
Hey Sami, This problem popped up after https://codereview.chromium.org/2787373002/ changed some timing, broke the test and ...
3 years, 8 months ago (2017-04-03 09:20:23 UTC) #3
Sami
lgtm, thanks! I was wondering if we could have this problem with loadEventFired elsewhere? At ...
3 years, 8 months ago (2017-04-03 10:10:44 UTC) #8
Eric Seckler
On 2017/04/03 10:10:44, Sami wrote: > lgtm, thanks! > > I was wondering if we ...
3 years, 8 months ago (2017-04-03 10:41:45 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2794883002/20001
3 years, 8 months ago (2017-04-03 10:43:11 UTC) #11
Sami
Right, readyState + current URL should avoid this (unless you're navigating to about:blank). headless_example.cc probably ...
3 years, 8 months ago (2017-04-03 10:44:11 UTC) #12
commit-bot: I haz the power
3 years, 8 months ago (2017-04-03 10:47:47 UTC) #15
Message was sent while issue was closed.
Committed patchset #1 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/bccb2a6db2d722ead168ce0244c4...

Powered by Google App Engine
This is Rietveld 408576698