New issue
Advanced search Search tips

Issue 838536 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug
Flaky-Test: DeclarativeNetRequestBrowserTest_Packed.PageWhitelistingAPI/0



Sign in to add a comment

DeclarativeNetRequestBrowserTest_Packed.PageWhitelistingAPI/0 is Flaky

Project Member Reported by Findit, May 1 2018

Issue description

Findit has detected a flake at test DeclarativeNetRequestBrowserTest_Packed.PageWhitelistingAPI/0.

Culprit (70.0% confidence): https://chromium-review.googlesource.com/q/I3438154f2231c202bd0f5a292905fc53ead02586
Regression range: None

Analysis: https://findit-for-me.appspot.com/waterfall/flake?key=ag9zfmZpbmRpdC1mb3ItbWVyxQELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9vdCKOAWNocm9taXVtLmNocm9taXVtb3MvbGludXgtY2hyb21lb3MtZGJnLzU0NTkvYnJvd3Nlcl90ZXN0cy9SR1ZqYkdGeVlYUnBkbVZPWlhSU1pYRjFaWE4wUW5KdmQzTmxjbFJsYzNSZlVHRmphMlZrTGxCaFoyVlhhR2wwWld4cGMzUnBibWRCVUVrdk1BPT0MCxITTWFzdGVyRmxha2VBbmFseXNpcxgBDA

If this result was incorrect, apply the label Findit-Incorrect-Result, mark the bug as Untriaged and the component Tools>Test>Findit>Flakiness.
 

Comment 1 by treib@chromium.org, May 3 2018

Labels: -Pri-3 OS-Chrome Pri-1
Owner: karandeepb@chromium.org
Status: Assigned (was: Available)
Culprit analysis is clearly wrong. I suspect https://chromium-review.googlesource.com/c/chromium/src/+/1034155, revert is in the CQ: https://chromium-review.googlesource.com/c/chromium/src/+/1041946
Project Member

Comment 2 by bugdroid1@chromium.org, May 3 2018

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

commit b87349ae1bb658b3d717da72a33611a5118f6a1f
Author: Marc Treib <treib@chromium.org>
Date: Thu May 03 11:28:15 2018

Revert "DNR tests: Use ExtensionTestMessageListener to detect background page load."

This reverts commit 6e6741a13720e1298a2d921540c265424155ffb3.

Reason for revert: DeclarativeNetRequestBrowserTest_Packed.PageWhitelistingAPI became flaky on ChromeOS debug and ASan/LSan:
https://ci.chromium.org/buildbot/chromium.memory/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/?limit=100
https://ci.chromium.org/buildbot/chromium.chromiumos/linux-chromeos-dbg/?limit=100
https://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=DeclarativeNetRequestBrowserTest_Packed.*PageWhitelist

Original change's description:
> DNR tests: Use ExtensionTestMessageListener to detect background page load.
>
> This CL changes declarative net request browsertests to use
> ExtensionTestMessageListener to detect background page load instead of relying
> on the notification system, which is deprecated. Also, add a
> extension_id_for_message() method to ExtensionTestMessageListener. This is
> useful to detect the extension id, the message was received from.
>
> BUG=696822
>
> Change-Id: Ie1fc74d547d4ee417c5f6b2d5a7082adbc9d164a
> Reviewed-on: https://chromium-review.googlesource.com/1034155
> Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
> Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#555005}

TBR=lazyboy@chromium.org,karandeepb@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 696822,  838536 
Change-Id: Ic714040d602db9e0a4abb266c0f84adb10029625
Reviewed-on: https://chromium-review.googlesource.com/1041946
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555691}
[modify] https://crrev.com/b87349ae1bb658b3d717da72a33611a5118f6a1f/chrome/browser/extensions/api/declarative_net_request/declarative_net_request_browsertest.cc
[modify] https://crrev.com/b87349ae1bb658b3d717da72a33611a5118f6a1f/extensions/common/api/declarative_net_request/test_utils.cc
[modify] https://crrev.com/b87349ae1bb658b3d717da72a33611a5118f6a1f/extensions/common/api/declarative_net_request/test_utils.h
[modify] https://crrev.com/b87349ae1bb658b3d717da72a33611a5118f6a1f/extensions/test/extension_test_message_listener.cc
[modify] https://crrev.com/b87349ae1bb658b3d717da72a33611a5118f6a1f/extensions/test/extension_test_message_listener.h

Comment 3 by treib@chromium.org, May 3 2018

Cc: karandeepb@chromium.org nedngu...@google.com
 Issue 838565  has been merged into this issue.
Cc: -nedngu...@google.com

Comment 5 by treib@chromium.org, May 3 2018

Labels: -Sheriff-Chromium
The
flakiness is most probably since we ended up creating the
ExtensionTestMessageListener too late for the
DeclarativeNetRequestBrowserTest_Packed.PageWhitelistingAPI test. The listener is created in SetUpOnMainThread override while the InstalledLoader ends up
loading all extensions before this (during BrowserMainLoop::CreateStartupTasks()).
Project Member

Comment 7 by bugdroid1@chromium.org, May 5 2018

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

commit c4eb630be5fdfef6fe21677d0403d74fd5e8c6ef
Author: Karan Bhatia <karandeepb@chromium.org>
Date: Sat May 05 06:46:07 2018

Reland: DNR tests: Use ExtensionTestMessageListener to detect background page load.

This CL relands r555005 which was reverted since it introduced flakiness. The
flakiness was most probably since we ended up creating the
ExtensionTestMessageListener too late for the
DeclarativeNetRequestBrowserTest_Packed.PageWhitelistingAPI test. The listener
was created in SetUpOnMainThread override while the InstalledLoader ends up
loading all extensions before this (during
BrowserMainLoop::CreateStartupTasks()). Hence a subsequent
ExtensionTestMessageListener::WaitUntilSatisfied() call would end up running
indefinitely. To get around this, only wait for the background page to load if
the background page isn't already ready.

BUG= 696822,  838536 

Change-Id: I2b288c6e6703e5be588d480403b362bf7c30db4e
Reviewed-on: https://chromium-review.googlesource.com/1043271
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556319}
[modify] https://crrev.com/c4eb630be5fdfef6fe21677d0403d74fd5e8c6ef/chrome/browser/extensions/api/declarative_net_request/declarative_net_request_browsertest.cc
[modify] https://crrev.com/c4eb630be5fdfef6fe21677d0403d74fd5e8c6ef/extensions/common/api/declarative_net_request/test_utils.cc
[modify] https://crrev.com/c4eb630be5fdfef6fe21677d0403d74fd5e8c6ef/extensions/common/api/declarative_net_request/test_utils.h
[modify] https://crrev.com/c4eb630be5fdfef6fe21677d0403d74fd5e8c6ef/extensions/test/extension_test_message_listener.cc
[modify] https://crrev.com/c4eb630be5fdfef6fe21677d0403d74fd5e8c6ef/extensions/test/extension_test_message_listener.h

Status: Fixed (was: Assigned)

Sign in to add a comment