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

Issue 701663 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Out until 24 Jan
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: ----
Type: ----



Sign in to add a comment

browser_tests failing on chromium.linux/Linux Tests

Project Member Reported by tzik@chromium.org, Mar 15 2017

Issue description

browser_tests failing on chromium.linux/Linux Tests

Builders failed on: 
- Linux Tests: 
  https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests

SubresourceFilterBrowserTest.FailedProvisionalLoadInMainframe is flaky after https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests/builds/53149.

Though I don't see clear culprit to this, https://codereview.chromium.org/2739193004 is relatively suspicious to me in the CLs in the range.

 

Comment 1 by tzik@chromium.org, Mar 15 2017

Cc: nasko@chromium.org

Comment 2 by tzik@chromium.org, Mar 15 2017

Cc: creis@chromium.org rdevlin....@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 15 2017

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

commit 96bb282f55edfa739e4e5f6633bb5fce61583e87
Author: tzik <tzik@chromium.org>
Date: Wed Mar 15 06:19:46 2017

Revert of Check for already existing entry when adding to SiteProcessMap. (patchset #7 id:160001 of https://codereview.chromium.org/2739193004/ )

Reason for revert:
Speculative revert to see a bot failure.

SubresourceFilterBrowserTest.FailedProvisionalLoadInMainframe got flaky around this CL, and this is relatively more suspicious in the failing range.

BUG= 701663 

Original issue's description:
> Check for already existing entry when adding to SiteProcessMap.
>
> The SiteProcessMap::RegisterProcess can overwrite existing entries
> in the map, which can lead to incorrect process being mapped to
> a site in process-per-site mode. This CL checks for existence and
> avoids the overwrite.
>
> BUG= 700514 
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
>
> Review-Url: https://codereview.chromium.org/2739193004
> Cr-Commit-Position: refs/heads/master@{#456920}
> Committed: https://chromium.googlesource.com/chromium/src/+/0e9d619f88ffabc3c237cbdf75603df8af9925e6

TBR=creis@chromium.org,rdevlin.cronin@chromium.org,nasko@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 700514 

Review-Url: https://codereview.chromium.org/2749543004
Cr-Commit-Position: refs/heads/master@{#457010}

[modify] https://crrev.com/96bb282f55edfa739e4e5f6633bb5fce61583e87/chrome/browser/extensions/process_management_browsertest.cc
[delete] https://crrev.com/f9d855c6039ca2daddbd4c3d08caac142fd15966/chrome/test/data/extensions/web_request_site_process_registration/blocked.html
[delete] https://crrev.com/f9d855c6039ca2daddbd4c3d08caac142fd15966/chrome/test/data/extensions/web_request_site_process_registration/manifest.json
[delete] https://crrev.com/f9d855c6039ca2daddbd4c3d08caac142fd15966/chrome/test/data/extensions/web_request_site_process_registration/test.html
[delete] https://crrev.com/f9d855c6039ca2daddbd4c3d08caac142fd15966/chrome/test/data/extensions/web_request_site_process_registration/test.js
[modify] https://crrev.com/96bb282f55edfa739e4e5f6633bb5fce61583e87/content/browser/frame_host/navigator_impl.cc
[modify] https://crrev.com/96bb282f55edfa739e4e5f6633bb5fce61583e87/content/browser/renderer_host/render_process_host_impl.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 15 2017

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

commit 0bcbed7d67f9a2813e1a278aada6c047a26bbbff
Author: nasko <nasko@chromium.org>
Date: Wed Mar 15 17:03:35 2017

Reland of Check for already existing entry when adding to SiteProcessMap. (patchset #1 id:1 of https://codereview.chromium.org/2749543004/ )

Reason for revert:
Reverting the revert to see if this was indeed the culprit for the flakiness.

Original issue's description:
> Revert of Check for already existing entry when adding to SiteProcessMap. (patchset #7 id:160001 of https://codereview.chromium.org/2739193004/ )
>
> Reason for revert:
> Speculative revert to see a bot failure.
>
> SubresourceFilterBrowserTest.FailedProvisionalLoadInMainframe got flaky around this CL, and this is relatively more suspicious in the failing range.
>
> BUG= 701663 
>
>
> Original issue's description:
> > Check for already existing entry when adding to SiteProcessMap.
> >
> > The SiteProcessMap::RegisterProcess can overwrite existing entries
> > in the map, which can lead to incorrect process being mapped to
> > a site in process-per-site mode. This CL checks for existence and
> > avoids the overwrite.
> >
> > BUG= 700514 
> > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
> >
> > Review-Url: https://codereview.chromium.org/2739193004
> > Cr-Commit-Position: refs/heads/master@{#456920}
> > Committed: https://chromium.googlesource.com/chromium/src/+/0e9d619f88ffabc3c237cbdf75603df8af9925e6
>
> TBR=creis@chromium.org,rdevlin.cronin@chromium.org,nasko@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= 700514 
>
> Review-Url: https://codereview.chromium.org/2749543004
> Cr-Commit-Position: refs/heads/master@{#457010}
> Committed: https://chromium.googlesource.com/chromium/src/+/96bb282f55edfa739e4e5f6633bb5fce61583e87

TBR=creis@chromium.org,rdevlin.cronin@chromium.org,tzik@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 701663 

Review-Url: https://codereview.chromium.org/2755643003
Cr-Commit-Position: refs/heads/master@{#457110}

[modify] https://crrev.com/0bcbed7d67f9a2813e1a278aada6c047a26bbbff/chrome/browser/extensions/process_management_browsertest.cc
[add] https://crrev.com/0bcbed7d67f9a2813e1a278aada6c047a26bbbff/chrome/test/data/extensions/web_request_site_process_registration/blocked.html
[add] https://crrev.com/0bcbed7d67f9a2813e1a278aada6c047a26bbbff/chrome/test/data/extensions/web_request_site_process_registration/manifest.json
[add] https://crrev.com/0bcbed7d67f9a2813e1a278aada6c047a26bbbff/chrome/test/data/extensions/web_request_site_process_registration/test.html
[add] https://crrev.com/0bcbed7d67f9a2813e1a278aada6c047a26bbbff/chrome/test/data/extensions/web_request_site_process_registration/test.js
[modify] https://crrev.com/0bcbed7d67f9a2813e1a278aada6c047a26bbbff/content/browser/frame_host/navigator_impl.cc
[modify] https://crrev.com/0bcbed7d67f9a2813e1a278aada6c047a26bbbff/content/browser/renderer_host/render_process_host_impl.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 15 2017

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

commit 7542ac912821eb3d5c939f17de433621d60f7fab
Author: nasko <nasko@chromium.org>
Date: Wed Mar 15 21:20:00 2017

Revert of Check for already existing entry when adding to SiteProcessMap. (patchset #1 id:1 of https://codereview.chromium.org/2755643003/ )

Reason for revert:
Test is flaking again, reverting the reland.

Original issue's description:
> Reland of Check for already existing entry when adding to SiteProcessMap. (patchset #1 id:1 of https://codereview.chromium.org/2749543004/ )
>
> Reason for revert:
> Reverting the revert to see if this was indeed the culprit for the flakiness.
>
> Original issue's description:
> > Revert of Check for already existing entry when adding to SiteProcessMap. (patchset #7 id:160001 of https://codereview.chromium.org/2739193004/ )
> >
> > Reason for revert:
> > Speculative revert to see a bot failure.
> >
> > SubresourceFilterBrowserTest.FailedProvisionalLoadInMainframe got flaky around this CL, and this is relatively more suspicious in the failing range.
> >
> > BUG= 701663 
> >
> >
> > Original issue's description:
> > > Check for already existing entry when adding to SiteProcessMap.
> > >
> > > The SiteProcessMap::RegisterProcess can overwrite existing entries
> > > in the map, which can lead to incorrect process being mapped to
> > > a site in process-per-site mode. This CL checks for existence and
> > > avoids the overwrite.
> > >
> > > BUG= 700514 
> > > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
> > >
> > > Review-Url: https://codereview.chromium.org/2739193004
> > > Cr-Commit-Position: refs/heads/master@{#456920}
> > > Committed: https://chromium.googlesource.com/chromium/src/+/0e9d619f88ffabc3c237cbdf75603df8af9925e6
> >
> > TBR=creis@chromium.org,rdevlin.cronin@chromium.org,nasko@chromium.org
> > # Skipping CQ checks because original CL landed less than 1 days ago.
> > NOPRESUBMIT=true
> > NOTREECHECKS=true
> > NOTRY=true
> > BUG= 700514 
> >
> > Review-Url: https://codereview.chromium.org/2749543004
> > Cr-Commit-Position: refs/heads/master@{#457010}
> > Committed: https://chromium.googlesource.com/chromium/src/+/96bb282f55edfa739e4e5f6633bb5fce61583e87
>
> TBR=creis@chromium.org,rdevlin.cronin@chromium.org,tzik@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= 701663 
>
> Review-Url: https://codereview.chromium.org/2755643003
> Cr-Commit-Position: refs/heads/master@{#457110}
> Committed: https://chromium.googlesource.com/chromium/src/+/0bcbed7d67f9a2813e1a278aada6c047a26bbbff

TBR=creis@chromium.org,rdevlin.cronin@chromium.org,tzik@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 701663 

Review-Url: https://codereview.chromium.org/2756573002
Cr-Commit-Position: refs/heads/master@{#457211}

[modify] https://crrev.com/7542ac912821eb3d5c939f17de433621d60f7fab/chrome/browser/extensions/process_management_browsertest.cc
[delete] https://crrev.com/3e36a03e908bfc7bf9a4335611ae4789280c1506/chrome/test/data/extensions/web_request_site_process_registration/blocked.html
[delete] https://crrev.com/3e36a03e908bfc7bf9a4335611ae4789280c1506/chrome/test/data/extensions/web_request_site_process_registration/manifest.json
[delete] https://crrev.com/3e36a03e908bfc7bf9a4335611ae4789280c1506/chrome/test/data/extensions/web_request_site_process_registration/test.html
[delete] https://crrev.com/3e36a03e908bfc7bf9a4335611ae4789280c1506/chrome/test/data/extensions/web_request_site_process_registration/test.js
[modify] https://crrev.com/7542ac912821eb3d5c939f17de433621d60f7fab/content/browser/frame_host/navigator_impl.cc
[modify] https://crrev.com/7542ac912821eb3d5c939f17de433621d60f7fab/content/browser/renderer_host/render_process_host_impl.cc

Labels: -Sheriff-Chromium
Labels: SafeBrowsing-Triaged
Owner: nasko@chromium.org
Status: Assigned (was: Available)

Comment 8 by nasko@chromium.org, Mar 24 2017

Status: Fixed (was: Assigned)
SubresourceFilterBrowserTest.FailedProvisionalLoadInMainframe flakiness was fixed in https://crrev.com/457796.

Sign in to add a comment