New issue
Advanced search Search tips

Issue 856330 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 9
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Regression



Sign in to add a comment

"external/wpt/service-workers/service-worker/about-blank-replacement.https.html" is flaky

Project Member Reported by chromium...@appspot.gserviceaccount.com, Jun 25 2018

Issue description

"external/wpt/service-workers/service-worker/about-blank-replacement.https.html" is flaky.

This issue was created automatically by the chromium-try-flakes app. Please find the right owner to fix the respective test/step and assign this issue to them. If the step/test is infrastructure-related, please add Infra-Troopers label and change issue status to Untriaged. When done, please remove the issue from Sheriff Bug Queue by removing the Sheriff-Chromium label.

We have detected 3 recent flakes. List of all flakes can be found at https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyWQsSBUZsYWtlIk5leHRlcm5hbC93cHQvc2VydmljZS13b3JrZXJzL3NlcnZpY2Utd29ya2VyL2Fib3V0LWJsYW5rLXJlcGxhY2VtZW50Lmh0dHBzLmh0bWwM.

Flaky tests should be disabled within 30 minutes unless culprit CL is found and reverted. Please see more details here: https://sites.google.com/a/chromium.org/dev/developers/tree-sheriffs/sheriffing-bug-queues#triaging-auto-filed-flakiness-bugs
 

Comment 1 by kbr@chromium.org, Jun 26 2018

 Issue 856338  has been merged into this issue.

Comment 2 by kbr@chromium.org, Jun 26 2018

 Issue 856133  has been merged into this issue.
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 26 2018

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

commit 26659aafba00ddb477fb7c05a7cda372e57192bc
Author: Kenneth Russell <kbr@chromium.org>
Date: Tue Jun 26 02:25:13 2018

Suppress flaky layout tests.

 fast/events/context-no-deselect.html
  - Suppress on all platforms, not just Windows.

 external/wpt/service-workers/service-worker/
    about-blank-replacement.https.html
  - Timing out in main and virtual test suites.

 http/tests/navigation/rename-subframe-goback.html
  - Fix linked bug ID.

Bug: 663847,  855816 ,  856330 
Change-Id: If2178c22714e64f6cd76267323f26634a18f9347
Tbr: alph@chromium.org
Tbr: tkent@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1114132
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570302}
[modify] https://crrev.com/26659aafba00ddb477fb7c05a7cda372e57192bc/third_party/WebKit/LayoutTests/TestExpectations

Project Member

Comment 4 by chromium...@appspot.gserviceaccount.com, Jun 27 2018

Detected 3 new flakes for test/step "virtual/service-worker-servicification/external/wpt/service-workers/service-worker/about-blank-replacement.https.html". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNygAELEgVGbGFrZSJ1dmlydHVhbC9zZXJ2aWNlLXdvcmtlci1zZXJ2aWNpZmljYXRpb24vZXh0ZXJuYWwvd3B0L3NlcnZpY2Utd29ya2Vycy9zZXJ2aWNlLXdvcmtlci9hYm91dC1ibGFuay1yZXBsYWNlbWVudC5odHRwcy5odG1sDA. This message was posted automatically by the chromium-try-flakes app.

Comment 5 by yigu@chromium.org, Jun 28 2018

 Issue 857410  has been merged into this issue.

Comment 6 by yigu@chromium.org, Jun 28 2018

Components: Blink>ServiceWorker
Labels: -Sheriff-Chromium
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 28 2018

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

commit ca733ee56c91374f42b50b5d5cef62a53cdcd152
Author: Yi Gu <yigu@chromium.org>
Date: Thu Jun 28 19:34:48 2018

Add virtual/service-worker-servicification/external/wpt/service-workers/service-worker/about-blank-replacement.https.html to TestExpectation

TBR=carlosk@chromium.org
NOTRY=true

Bug:  856330 
Change-Id: Id82dc29562058664dc85265c3ddd1ed9559d988d
Reviewed-on: https://chromium-review.googlesource.com/1119085
Reviewed-by: Yi Gu <yigu@chromium.org>
Commit-Queue: Yi Gu <yigu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571225}
[modify] https://crrev.com/ca733ee56c91374f42b50b5d5cef62a53cdcd152/third_party/WebKit/LayoutTests/TestExpectations

Comment 8 by falken@chromium.org, Jun 29 2018

Labels: -Type-Bug Type-Bug-Regression
Owner: leon....@intel.com
Status: Assigned (was: Untriaged)
Leon: this has the same cause as   issue 856969 .

Comment 9 by leon....@intel.com, Jun 29 2018

Status: Started (was: Assigned)
Locally confirmed about-blank-replacement.https.html can pass 100 iterations with the fix CL https://chromium-review.googlesource.com/c/chromium/src/+/1118006 of  issue 856969 .
Project Member

Comment 10 by bugdroid1@chromium.org, Jun 29 2018

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

commit 1c81791b45c09b52614f85a7016425b71da32c4d
Author: Han Leon <leon.han@intel.com>
Date: Fri Jun 29 07:11:10 2018

[ServiceWorker] Stop observing EmbeddedWorkerInstance once it's gone away

This CL adds a function OnDestroyed() into
EmbeddedWorkerInstance::Listener interface, so that
ServiceWorkerRegisterJob can stop observing EmbeddedWorkerInstance once
it's gone away.

BUG= 856969 , 856330 

Change-Id: I3acc9143f9ea2f8f24ab0aac465f29f97fdf9906
Reviewed-on: https://chromium-review.googlesource.com/1118006
Commit-Queue: Han Leon <leon.han@intel.com>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571417}
[modify] https://crrev.com/1c81791b45c09b52614f85a7016425b71da32c4d/content/browser/service_worker/embedded_worker_instance.cc
[modify] https://crrev.com/1c81791b45c09b52614f85a7016425b71da32c4d/content/browser/service_worker/embedded_worker_instance.h
[modify] https://crrev.com/1c81791b45c09b52614f85a7016425b71da32c4d/content/browser/service_worker/service_worker_register_job.cc
[modify] https://crrev.com/1c81791b45c09b52614f85a7016425b71da32c4d/content/browser/service_worker/service_worker_register_job.h
[modify] https://crrev.com/1c81791b45c09b52614f85a7016425b71da32c4d/third_party/WebKit/LayoutTests/TestExpectations

Project Member

Comment 11 by chromium...@appspot.gserviceaccount.com, Jun 29 2018

Labels: Sheriff-Chromium
Detected 3 new flakes for test/step "virtual/outofblink-cors/external/wpt/service-workers/service-worker/about-blank-replacement.https.html". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNycQsSBUZsYWtlImZ2aXJ0dWFsL291dG9mYmxpbmstY29ycy9leHRlcm5hbC93cHQvc2VydmljZS13b3JrZXJzL3NlcnZpY2Utd29ya2VyL2Fib3V0LWJsYW5rLXJlcGxhY2VtZW50Lmh0dHBzLmh0bWwM. This message was posted automatically by the chromium-try-flakes app. Since flakiness is ongoing, the issue was moved back into Sheriff Bug Queue (unless already there).
Labels: -Sheriff-Chromium
Project Member

Comment 13 by bugdroid1@chromium.org, Jul 7

Labels: merge-merged-3440
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/baf2459c1cc305b127d41424a86d948e731ee233

commit baf2459c1cc305b127d41424a86d948e731ee233
Author: Han Leon <leon.han@intel.com>
Date: Sat Jul 07 06:20:28 2018

[M68] [ServiceWorker] Stop observing EmbeddedWorkerInstance once it's gone away

This CL is a combination of the 2 previous commits on trunk, both of
them aim to avoid use-after-free behavior on the raw pointer of an
EmbeddedWorkerInstance.
After the 1st CL landed on trunk, it turns out that it did not fix the
problem completely and caused regressions like  issue 856969 , then we
have the 2nd CL give a complete solution.

The 1st one:
"
[ServiceWorker] Avoid using stale pointer of EmbeddedWorkerInstance

We're saving the raw pointer of an EmbeddedWorkerInstance in a
base::ScopedObserver member of ServiceWorkerRegisterJob, but, the
EmbeddedWorkerInstance is not guaranteed to outlive the
ServiceWorkerRegisterJob, it may get destroyed and lead to a situation
that ServiceWorkerRegisterJob holds a stale pointer, which could lead
to crash.

This CL lets ServiceWorkerRegisterJob listen OnDetached() from the
EmbeddedWorkerInstance so it can be aware that the EmbeddedWorkerInstanc
pointer is going to be stale and should be removed from the source list
of base::ScopedObserver.

This fix should be safe enough now, but in longer term we'll try to
remove EmbeddedWorkerInstance::Listener interface and let
EmbeddedWorkerInstance talk directly to its owner ServiceWorkerVersion,
then let ServiceWorkerRegisterJob observe the ServiceWorkerVersion it's
manipulating to get OnScriptLoaded() notification.

BUG=854063,855394

Change-Id: I9c5a46beda1aafff32a86b9055be2b53d50fda97
Reviewed-on: https://chromium-review.googlesource.com/1112904
Commit-Queue: Han Leon <leon.han@intel.com>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569906}
(cherry picked from commit 26f26cd82a8159dff52fdef10b0a7ebb84c48040)
"

The 2nd one:
"
[ServiceWorker] Stop observing EmbeddedWorkerInstance once it's gone away

This CL adds a function OnDestroyed() into
EmbeddedWorkerInstance::Listener interface, so that
ServiceWorkerRegisterJob can stop observing EmbeddedWorkerInstance once
it's gone away.

BUG= 856969 , 856330 

Change-Id: I3acc9143f9ea2f8f24ab0aac465f29f97fdf9906
Reviewed-on: https://chromium-review.googlesource.com/1118006
Commit-Queue: Han Leon <leon.han@intel.com>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571417}
(cherry picked from commit 1c81791b45c09b52614f85a7016425b71da32c4d)
"

BUG=854063, 856969 

Change-Id: I91c68d8edba41f6f7725903288fd12ec7574c538
Reviewed-on: https://chromium-review.googlesource.com/1128469
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#608}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/baf2459c1cc305b127d41424a86d948e731ee233/content/browser/service_worker/embedded_worker_instance.cc
[modify] https://crrev.com/baf2459c1cc305b127d41424a86d948e731ee233/content/browser/service_worker/embedded_worker_instance.h
[modify] https://crrev.com/baf2459c1cc305b127d41424a86d948e731ee233/content/browser/service_worker/service_worker_register_job.cc
[modify] https://crrev.com/baf2459c1cc305b127d41424a86d948e731ee233/content/browser/service_worker/service_worker_register_job.h

Status: Fixed (was: Started)

Sign in to add a comment