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

Issue 869553 link

Starred by 1 user
Status: Fixed
Owner:
█ roc...@chromium.org
please use my google.com address
Closed: Aug 2
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 866708



Sign in to add a comment

Some storage unit tests fail when changing Mojo C++ bindings dispatch scheduling

Project Member Reported by roc...@chromium.org, Jul 31 
When attempting to land this CL[1], the following content_unittests tests break for unknown (not yet investigated) reasons:

LocalStorageContextMojoTestWithService.RecreateOnCommitFailure
SessionStorageContextMojoTest.RecreateOnCommitFailure
StorageAreaImplTest/StorageAreaImplParamTest.PrefixForkAfterLoad/1
StorageAreaImplTest/StorageAreaImplParamTest.PrefixForkAfterLoad/0

This shouldn't be possible since the bindings change does not violate any guarantees made by Mojo APIs. Most likely a subtle timing issue, wrong expectations, etc.

These will need to be fixed before the CL can be landed.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1145692
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 2 

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

commit a32641c8a3c25698b55b61761b2730efcf5c7eb7
Author: Ken Rockot <rockot@chromium.org>
Date: Thu Aug 02 04:20:14 2018

Fix some storage unit tests

A few of these tests have some subtle scheduling issues which are
hit when we attempt to turn on more granular Mojo bindings message
dispatch.

This CL adds some synchronization via FlushForTesting calls where
appropriate to avoid such issues.

TBR=sky@chromium.org

Bug:  869553 
Change-Id: I8d4d5be6f670db3b3e369f70dfd987dc0b1f1ff2
Reviewed-on: https://chromium-review.googlesource.com/1159591
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580059}
[modify] https://crrev.com/a32641c8a3c25698b55b61761b2730efcf5c7eb7/content/browser/dom_storage/local_storage_context_mojo_unittest.cc
[modify] https://crrev.com/a32641c8a3c25698b55b61761b2730efcf5c7eb7/content/browser/dom_storage/session_storage_context_mojo_unittest.cc
[modify] https://crrev.com/a32641c8a3c25698b55b61761b2730efcf5c7eb7/content/browser/dom_storage/storage_area_impl_unittest.cc
[modify] https://crrev.com/a32641c8a3c25698b55b61761b2730efcf5c7eb7/content/browser/dom_storage/test/fake_leveldb_service.cc
[modify] https://crrev.com/a32641c8a3c25698b55b61761b2730efcf5c7eb7/content/browser/dom_storage/test/fake_leveldb_service.h
[modify] https://crrev.com/a32641c8a3c25698b55b61761b2730efcf5c7eb7/content/test/fake_leveldb_database.cc
[modify] https://crrev.com/a32641c8a3c25698b55b61761b2730efcf5c7eb7/content/test/fake_leveldb_database.h

Comment 2 by roc...@chromium.org, Aug 2

Status: Fixed (was: Assigned)
►

Sign in to add a comment