Fix race condition when resolving performanceTiming.loadEventEnd#22812
Fix race condition when resolving performanceTiming.loadEventEnd#22812zhouyx merged 4 commits intoampproject:masterfrom
Conversation
src/service/variable-source.js
Outdated
| export function getTimingDataAsync(win, startEvent, endEvent) { | ||
| return loadPromise(win).then(() => { | ||
| const deferred = new Deferred(); | ||
| loadPromise(win).then(() => { |
There was a problem hiding this comment.
would the following work?
loadPromise(win)
.then(timer.promise(1))
.then(...);There was a problem hiding this comment.
That's should work, as long as there's a setTimeout call involved. But do you think it's worth retrieving the Timer service here?
There was a problem hiding this comment.
Yep, the whole point of timer service is to share code.
There was a problem hiding this comment.
BTW: I found a delayAfterDeferringToEventLoop method from the lightbox-gallery component and few use case of timer.promise(). May worth introducing a global helper method for this. But I'll stick to timer.promise here.
lannka
left a comment
There was a problem hiding this comment.
any idea for a test? would it be easy to add an integration test? (check the value is never 0)
|
I couldn't reproduce the same race condition with our integration test fixture. Let me know if you still feel it's worth adding that. |
|
Since you already have it, let's get it in. I think there is still value for it. |
|
integration test added |
|
Thanks for the fix! |
No description provided.