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

Issue 922732 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 18
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Fuchsia
Pri: 1
Type: Bug

Blocking:
issue 905513



Sign in to add a comment

ChildProcessSecurityPolicyTest.DynamicIsolatedOrigins fails on Fuchsia x64

Project Member Reported by sergeybe...@chromium.org, Jan 16 (6 days ago)

Issue description

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Fuchsia%20x64

First failing build: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Fuchsia%20x64/27511

Snippet of log that contains the failure.

[ RUN      ] ChildProcessSecurityPolicyTest.DynamicIsolatedOrigins
../../content/browser/child_process_security_policy_unittest.cc:1391: Failure
Expected equality of these values:
  BrowsingInstanceId::FromUnsafeValue(1)
    Which is: 1
  SiteInstanceImpl::NextBrowsingInstanceId()
    Which is: 2
Stack trace:
#00: testing::internal::UnitTestImpl::CurrentOsStackTraceExceptTop(int) at gtest.cc:?
#01: testing::internal::AssertHelper::operator=(testing::Message const&) const at gtest.cc:?
#02: content::ChildProcessSecurityPolicyTest_DynamicIsolatedOrigins_Test::TestBody() at child_process_security_policy_unittest.cc:?
[00254.664] pkgsvr: pkgfs:unsupported(/packages/content_unittests/0): dir unlink "content_unittests__exec.log"

../../content/browser/child_process_security_policy_unittest.cc:1398: Failure

[...]

1 test failed:
    ChildProcessSecurityPolicyTest.DynamicIsolatedOrigins (../../content/browser/child_process_security_policy_unittest.cc:1376)

 

Comment 1 by sergeybe...@chromium.org, Jan 16 (6 days ago)

This is paging the trooper and causes overload on a CQ trybot https://ci.chromium.org/p/chromium/builders/luci.chromium.try/fuchsia_x64 due to retries without patch.

Comment 2 by sergeybe...@chromium.org, Jan 16 (6 days ago)

Cc: slightlyoff@chromium.org ksakamoto@chromium.org iclell...@chromium.org bever...@google.com
+current sheriffs FYI

Comment 3 by ksakamoto@chromium.org, Jan 17 (6 days ago)

Could not find clear culprit in the build range. I'm disabling it on Fuchsia.
https://chromium-review.googlesource.com/c/chromium/src/+/1415052

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 17 (6 days ago)

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

commit 97250d12107aa37ca1fcc68b6d0d252525d0aaaf
Author: Kunihiko Sakamoto <ksakamoto@chromium.org>
Date: Thu Jan 17 01:20:38 2019

Disable ChildProcessSecurityPolicyTest.DynamicIsolatedOrigins on Fuchsia

TBR=alexmos@chromium.org,creis@chromium.org,lukasza@chromium.org

No-Try: true
Bug:  922732 
Change-Id: Ic3d35731508e7d8e1365e317a31a787526a3e220
Reviewed-on: https://chromium-review.googlesource.com/c/1415052
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623496}
[modify] https://crrev.com/97250d12107aa37ca1fcc68b6d0d252525d0aaaf/content/browser/child_process_security_policy_unittest.cc

Comment 5 by ksakamoto@chromium.org, Jan 17 (6 days ago)

Blocking: 905513
Owner: alex...@chromium.org
Status: Assigned (was: Untriaged)
Assigning alexmos@ (author of the test case) for further investigation.

Comment 6 by fdegans@chromium.org, Jan 17 (6 days ago)

Cc: alex...@chromium.org
 Issue 922763  has been merged into this issue.

Comment 7 by alex...@chromium.org, Jan 18 (5 days ago)

Cc: w...@chromium.org fdegans@chromium.org creis@chromium.org
The failure logs indicate that when this tests runs on Fuchsia, something has already created a BrowsingInstance and bumped the global ID counter before the test starts to run.  That could happen from a navigation or creating a SiteInstance.  Anyone know if there's any extra setup that's done for content_unittests only for Fuchsia?  Or maybe a SiteInstance leaks from a previous test?

I suppose I could change the test to not depend on the starting BrowsingInstance having ID 1, though it'd be nice to understand where the extra startup BrowsingInstance is coming from, and why it's not present on other platforms.

Comment 8 by w...@chromium.org, Jan 18 (4 days ago)

Re #7: We run tests in batches of ten tests per batch-process, by default, so if you're depending on nothing every having created a BrowsingInstance in the same process in this test then yes, the test should be updated to tolerate running in a process that has already run other tests. :)

Comment 9 by w...@chromium.org, Jan 18 (4 days ago)

Labels: M-73

Comment 10 by alex...@chromium.org, Jan 18 (4 days ago)

Status: Started (was: Assigned)
Yup, I'll go ahead and update the test.  I see that there's only one other ChildProcessSecurityPolicyTest that creates a SiteInstance and hence BrowsingInstance,  ChildProcessSecurityPolicyTest.CanAccessDataForOrigin (added shortly after my test), and on the Fuchsia bot it ran just before the DynamicIsolatedOrigins test, likely in the same batch.

Comment 11 by w...@chromium.org, Jan 18 (4 days ago)

Summary: ChildProcessSecurityPolicyTest.DynamicIsolatedOrigins fails on Fuchsia x64 (was: content_unittests are broken on Fuchsia x64 builder)
Project Member

Comment 12 by bugdroid1@chromium.org, Jan 18 (4 days ago)

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

commit 67bc07353c83b1d85e51a4acb97214ef6f41f92a
Author: Alex Moshchuk <alexmos@chromium.org>
Date: Fri Jan 18 21:23:33 2019

Fix flakiness in ChildProcessSecurityPolicyTest.DynamicIsolatedOrigins

This test assumed that the initial BrowsingInstance ID will always be
1, but this won't hold if another unit test ran before this test in
the same batch and created a BrowsingInstance that bumped up the
global ID counter.  This CL changes the test to look up the first
BrowsingInstanceID and use it as a base for the other relevant checks.

Bug:  922732 
Change-Id: I037acc06144032b29e021e22f1fdd2cdc714f4a2
Reviewed-on: https://chromium-review.googlesource.com/c/1422499
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624285}
[modify] https://crrev.com/67bc07353c83b1d85e51a4acb97214ef6f41f92a/content/browser/child_process_security_policy_unittest.cc

Comment 13 by w...@chromium.org, Jan 18 (4 days ago)

Status: Fixed (was: Started)

Sign in to add a comment