Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(23)

Issue 1139533006: Revert of Be explicit about forcing TouchSelectionController updates (Closed)

Created:
5 years, 7 months ago by benwells
Modified:
5 years, 7 months ago
Reviewers:
jdduke (slow), mohsen
CC:
chromium-reviews, Donn Denman
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Be explicit about forcing TouchSelectionController updates (patchset #2 id:20001 of https://codereview.chromium.org/1127383007/) Reason for revert: This has caused failures on the memory bots. e.g.:http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20MSan%20Tests/builds/6179 Sample test output: [ RUN ] TouchSelectionControllerTest.AllowShowingFromCurrentSelection ==6485== WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x7f5bc93bb605 in ui::TouchSelectionController::OnSelectionBoundsChanged(ui::SelectionBound const&, ui::SelectionBound const&) ui/touch_selection/touch_selection_controller.cc:71:27 #1 0x7f5bc922803a in ui::TouchSelectionControllerTest::ChangeSelection(gfx::RectF const&, bool, gfx::RectF const&, bool) ui/touch_selection/touch_selection_controller_unittest.cc:147:5 #2 0x7f5bc9278c8d in ui::TouchSelectionControllerTest_AllowShowingFromCurrentSelection_Test::TestBody() ui/touch_selection/touch_selection_controller_unittest.cc:946:3 #3 0x7f5bc93631dc in HandleExceptionsInMethodIfSupported\u003Ctesting::Test, void> testing/gtest/src/gtest.cc:2420:12 #4 0x7f5bc93631dc in testing::Test::Run() testing/gtest/src/gtest.cc:2436:0 #5 0x7f5bc9365a4c in testing::TestInfo::Run() testing/gtest/src/gtest.cc:2612:5 #6 0x7f5bc93673f6 in testing::TestCase::Run() testing/gtest/src/gtest.cc:2730:5 #7 0x7f5bc9384d03 in testing::internal::UnitTestImpl::RunAllTests() testing/gtest/src/gtest.cc:4602:11 #8 0x7f5bc9383ce2 in HandleExceptionsInMethodIfSupported\u003Ctesting::internal::UnitTestImpl, bool> testing/gtest/src/gtest.cc:2420:12 #9 0x7f5bc9383ce2 in testing::UnitTest::Run() testing/gtest/src/gtest.cc:4220:0 #10 0x7f5bc93078a3 in RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2326:10 #11 0x7f5bc93078a3 in base::TestSuite::Run() base/test/test_suite.cc:228:0 #12 0x7f5bc92f7b2d in (anonymous namespace)::RunTestSuite(int, char**) base/test/run_all_unittests.cc:25:10 #13 0x7f5bc92f85ee in Run base/callback.h:396:12 #14 0x7f5bc92f85ee in base::(anonymous namespace)::LaunchUnitTestsInternal(base::Callback\u003Cint ()> const&, int, bool, base::Callback\u003Cvoid ()> const&) base/test/launcher/unit_test_launcher.cc:184:0 #15 0x7f5bc92f7f5b in base::LaunchUnitTests(int, char**, base::Callback\u003Cint ()> const&) base/test/launcher/unit_test_launcher.cc:423:10 #16 0x7f5bc92f792d in main base/test/run_all_unittests.cc:37:10 #17 0x7f5bc206a76c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226:0 #18 0x7f5bc9188f38 in _start ??:0:0 Uninitialized value was created by a heap allocation #0 0x7f5bc91da0e2 in operator new(unsigned long) ??:0:0 #1 0x7f5bc927c95c in ui::TouchSelectionControllerTest::SetUp() ui/touch_selection/touch_selection_controller_unittest.cc:65:23 #2 0x7f5bc9362f57 in HandleExceptionsInMethodIfSupported\u003Ctesting::Test, void> testing/gtest/src/gtest.cc:2420:12 #3 0x7f5bc9362f57 in testing::Test::Run() testing/gtest/src/gtest.cc:2432:0 #4 0x7f5bc9365a4c in testing::TestInfo::Run() testing/gtest/src/gtest.cc:2612:5 #5 0x7f5bc93673f6 in testing::TestCase::Run() testing/gtest/src/gtest.cc:2730:5 #6 0x7f5bc9384d03 in testing::internal::UnitTestImpl::RunAllTests() testing/gtest/src/gtest.cc:4602:11 #7 0x7f5bc9383ce2 in HandleExceptionsInMethodIfSupported\u003Ctesting::internal::UnitTestImpl, bool> testing/gtest/src/gtest.cc:2420:12 #8 0x7f5bc9383ce2 in testing::UnitTest::Run() testing/gtest/src/gtest.cc:4220:0 #9 0x7f5bc93078a3 in RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2326:10 #10 0x7f5bc93078a3 in base::TestSuite::Run() base/test/test_suite.cc:228:0 #11 0x7f5bc92f7b2d in (anonymous namespace)::RunTestSuite(int, char**) base/test/run_all_unittests.cc:25:10 #12 0x7f5bc92f85ee in Run base/callback.h:396:12 #13 0x7f5bc92f85ee in base::(anonymous namespace)::LaunchUnitTestsInternal(base::Callback\u003Cint ()> const&, int, bool, base::Callback\u003Cvoid ()> const&) base/test/launcher/unit_test_launcher.cc:184:0 #14 0x7f5bc92f7f5b in base::LaunchUnitTests(int, char**, base::Callback\u003Cint ()> const&) base/test/launcher/unit_test_launcher.cc:423:10 #15 0x7f5bc92f792d in main base/test/run_all_unittests.cc:37:10 #16 0x7f5bc206a76c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226:0 SUMMARY: MemorySanitizer: use-of-uninitialized-value (/mnt/data/b/build/slave/Linux_MSan_Tests/build/src/out/Release/ui_touch_selection_unittests+0x285605) Exiting Original issue's description: > Be explicit about forcing TouchSelectionController updates > > Previously, cached values in the TouchSelectionController would be reset > to force future selection updates. However, these cached values can > actually be used outside of selection update calls, e.g., when force > showing the selection from the current cached values. Instead of > resetting the cached values, simply set a dirty bit that forces an > update, avoiding issues when dealing with the reset values. > > BUG=393025 > > Committed: https://crrev.com/fdcf817da49ee92fe191981f7525503444f75f83 > Cr-Commit-Position: refs/heads/master@{#329325} TBR=mohsen@chromium.org,jdduke@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=393025 Committed: https://crrev.com/e63eab6ae8072fc359f151e245e07d4864035ba2 Cr-Commit-Position: refs/heads/master@{#329352}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -19 lines) Patch
M ui/touch_selection/touch_selection_controller.h View 2 chunks +1 line, -5 lines 0 comments Download
M ui/touch_selection/touch_selection_controller.cc View 7 chunks +14 lines, -11 lines 0 comments Download
M ui/touch_selection/touch_selection_controller_unittest.cc View 1 chunk +0 lines, -3 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
benwells
Created Revert of Be explicit about forcing TouchSelectionController updates
5 years, 7 months ago (2015-05-12 04:51:18 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1139533006/1
5 years, 7 months ago (2015-05-12 04:51:57 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 7 months ago (2015-05-12 04:52:59 UTC) #3
commit-bot: I haz the power
5 years, 7 months ago (2015-05-12 04:53:45 UTC) #4
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/e63eab6ae8072fc359f151e245e07d4864035ba2
Cr-Commit-Position: refs/heads/master@{#329352}

Powered by Google App Engine
This is Rietveld 408576698