New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 788372 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

LocalNTPTest::NavigateToNTPAndWaitUntilLoaded is racy

Project Member Reported by treib@chromium.org, Nov 24 2017

Issue description

NavigateToNTPAndWaitUntilLoaded does the following:
- Attach a DOMMessageQueue to the current tab.
- Navigate to an NTP.
- Register a window event listener for the 'loaded' message from the iframe.
- Wait for that message.

However, it can happen that the 'loaded' message arrives before the event listener is registered, in which case we'll wait forever. This causes tests that use this function (LocalNTPTest.*GoogleNTPLoadsWithoutError) to occasionally time out.
 

Comment 1 by treib@chromium.org, Nov 24 2017

Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d164003ca3d8accad544d6cedc5b85cbbc45262e

commit d164003ca3d8accad544d6cedc5b85cbbc45262e
Author: Marc Treib <treib@chromium.org>
Date: Tue Nov 28 15:18:05 2017

Local NTP tests: Fix NavigateToNTPAndWaitUntilLoaded

The previous implementation was racy and sometimes missed the 'loaded'
message from the iframe, causing tests to timeout.

Bug:  788372 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I0052aa03f451ba295475b7460565e4e3f88bf3b2
Reviewed-on: https://chromium-review.googlesource.com/788954
Reviewed-by: Chris Pickel <sfiera@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519691}
[modify] https://crrev.com/d164003ca3d8accad544d6cedc5b85cbbc45262e/chrome/browser/resources/local_ntp/local_ntp.js
[modify] https://crrev.com/d164003ca3d8accad544d6cedc5b85cbbc45262e/chrome/browser/resources/local_ntp/most_visited_single.js
[modify] https://crrev.com/d164003ca3d8accad544d6cedc5b85cbbc45262e/chrome/browser/ui/search/local_ntp_browsertest.cc

Comment 3 by treib@chromium.org, Nov 28 2017

Status: Fixed (was: Started)

Sign in to add a comment