New issue
Advanced search Search tips

Issue 691469 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug



Sign in to add a comment

Fix TabHistoryPopupControllerTestCase testTabHistoryMenu

Project Member Reported by olivierrobin@chromium.org, Feb 13 2017

Issue description

Test fail because it compares CRWSessionEntry instead of comparing the inner NavigationItem.

If the suggested fix seems reasonable, please close this bug.
If not, please revert the temporary fix but ensure the test pass (or disable it).

 
Another possible fix is to remove the DCHECK([sessionController.entries containsObject:entry]); in tab.mm:1295
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 13 2017

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

commit b8d957c64f413bde7d4e32bdf375bf622446e7ff
Author: olivierrobin <olivierrobin@chromium.org>
Date: Mon Feb 13 10:25:53 2017

Add CRWSessionEntry isEqual

EGTest have DCHECKs on CRWSessionEntry equality (e.g.
TabHistoryPopupControllerTestCase testTabHistoryMenu).
This fails because entry can be different even if they have the same
navigation item.

This is a tentative fix by the sheriff to make the tree green.
Please review this CL and revert it later if there is a better solution.

BUG= 691469 

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

[modify] https://crrev.com/b8d957c64f413bde7d4e32bdf375bf622446e7ff/ios/web/navigation/crw_session_entry.mm

Failing test
https://build.chromium.org/p/chromium.fyi/builders/EarlGreyiOS/builds/24238

Stack

Test Suite 'TabHistoryPopupControllerTestCase' started at 2017-02-13 02:23:06.291
[INFO] GCDWebServer started on port 8080 and reachable at http://192.168.146.38:8080/
Test Case '-[TabHistoryPopupControllerTestCase testTabHistoryMenu]' started.
[0213/022310.656881:FATAL:tab.mm(1295)] Check failed: [sessionController.entries containsObject:entry].
0   ios_chrome_ui_egtests               0x00000001044876fd base::debug::StackTrace::StackTrace(unsigned long) + 157
1   ios_chrome_ui_egtests               0x000000010448777d base::debug::StackTrace::StackTrace(unsigned long) + 29
2   ios_chrome_ui_egtests               0x000000010448643c base::debug::StackTrace::StackTrace() + 28
3   ios_chrome_ui_egtests               0x00000001044f4ee0 logging::LogMessage::~LogMessage() + 80
4   ios_chrome_ui_egtests               0x00000001044f2bd5 logging::LogMessage::~LogMessage() + 21
5   ios_chrome_ui_egtests               0x00000001041f0706 -[Tab goToEntry:] + 582
6   ios_chrome_ui_egtests               0x0000000104031036 -[BrowserViewController navigateToSelectedEntry:] + 406
7   ios_chrome_ui_egtests               0x0000000104035978 -[BrowserViewController chromeExecuteCommand:] + 4392
8   ios_chrome_ui_egtests               0x00000001039f9e08 -[UIResponder(ChromeExecuteCommand) chromeExecuteCommand:] + 104
9   ios_chrome_ui_egtests               0x00000001039f9e08 -[UIResponder(ChromeExecuteCommand) chromeExecuteCommand:] + 104
10  ios_chrome_ui_egtests               0x00000001039f9e08 -[UIResponder(ChromeExecuteCommand) chromeExecuteCommand:] + 104
11  ios_chrome_ui_egtests               0x00000001039f9e08 -[UIResponder(ChromeExecuteCommand) chromeExecuteCommand:] + 104
12  ios_chrome_ui_egtests               0x00000001039f9e08 -[UIResponder(ChromeExecuteCommand) chromeExecuteCommand:] + 104
13  ios_chrome_ui_egtests               0x0000000103dc9583 -[TabHistoryViewController collectionView:didSelectItemAtIndexPath:] + 131
14  UIKit                               0x000000010f7dfdf4 -[UICollectionView _selectItemAtIndexPath:animated:scrollPosition:notifyDelegate:] + 702
15  UIKit                               0x000000010f80a86e -[UICollectionView touchesEnded:withEvent:] + 665
16  UIKit                               0x000000010f0b9c1b forwardTouchMethod + 348
17  UIKit                               0x000000010f0b9ce4 -[UIResponder touchesEnded:withEvent:] + 49
18  UIKit                               0x000000010f0b9c1b forwardTouchMethod + 348
19  UIKit                               0x000000010f0b9ce4 -[UIResponder touchesEnded:withEvent:] + 49
20  UIKit                               0x000000010f3db1dc _UIGestureEnvironmentSortAndSendDelayedTouches + 5645
21  UIKit                               0x000000010f3d5ea3 _UIGestureEnvironmentUpdate + 1472
22  UIKit                               0x000000010f3d589b -[UIGestureEnvironment _deliverEvent:toGestureRecognizers:usingBlock:] + 521
23  UIKit                               0x000000010f3d4a7e -[UIGestureEnvironment _updateGesturesForEvent:window:] + 286
24  UIKit                               0x000000010ef137ad -[UIWindow sendEvent:] + 3989
25  UIKit                               0x000000010eec0a33 -[UIApplication sendEvent:] + 371
26  UIKit                               0x00000001279f6959 -[UIApplicationAccessibility sendEvent:] + 93
27  EarlGrey                            0x000000010ecfad18 -[GREYTouchInjector grey_injectTouches:] + 1496
28  EarlGrey                            0x000000010ecf9f71 -[GREYTouchInjector timerFiredWithZeroToleranceTimer:] + 849
29  EarlGrey                            0x000000010ecfbe95 __50-[GREYZeroToleranceTimer initWithInterval:target:]_block_invoke + 53
30  EarlGrey                            0x000000010ecfc3a7 __78+[GREYZeroToleranceTimer grey_scheduleZeroToleranceTimerWithInterval:handler:]_block_invoke + 39
31  libdispatch.dylib                   0x000000011662112e _dispatch_client_callout + 8
32  libdispatch.dylib                   0x00000001166049d5 _dispatch_continuation_pop + 578
33  libdispatch.dylib                   0x000000011661577d _dispatch_source_latch_and_call + 192
34  libdispatch.dylib                   0x000000011660e5a9 _dispatch_source_invoke + 1259
35  libdispatch.dylib                   0x000000011660833e _dispatch_main_queue_callback_4CF + 559
36  CoreFoundation                      0x000000010d7944f9 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 9
37  CoreFoundation                      0x000000010d759f8d __CFRunLoopRun + 2205
38  CoreFoundation                      0x000000010d759494 CFRunLoopRunSpecific + 420
39  EarlGrey                            0x000000010ed13d7f -[GREYRunLoopSpinner grey_drainRunLoopInActiveModeAndCheckCondition:forTime:] + 687
40  EarlGrey                            0x000000010ed1372d -[GREYRunLoopSpinner spinWithStopConditionBlock:] + 1117
41  EarlGrey                            0x000000010ecf9b4e -[GREYTouchInjector waitUntilAllTouchesAreDeliveredUsingInjector] + 846
42  EarlGrey                            0x000000010ecf8d60 -[GREYSyntheticEvents grey_endTouchesAtPoints:timeElapsedSinceLastTouchDelivery:] + 176
43  EarlGrey                            0x000000010ecf7d0a +[GREYSyntheticEvents touchAlongMultiplePaths:relativeToWindow:forDuration:expendable:] + 1098
44  EarlGrey                            0x000000010ecf786b +[GREYSyntheticEvents touchAlongPath:relativeToWindow:forDuration:expendable:] + 219
45  EarlGrey                            0x000000010ecb973f +[GREYTapper tapOnWindow:numberOfTaps:location:error:] + 399
46  EarlGrey                            0x000000010ecb955d +[GREYTapper tapOnElement:numberOfTaps:location:error:] + 909
47  EarlGrey                            0x000000010ecb87a3 -[GREYTapAction perform:error:] + 371
48  EarlGrey                            0x000000010eceaae0 __46-[GREYElementInteraction performAction:error:]_block_invoke + 1056
49  EarlGrey                            0x000000010ed17eb4 __59-[GREYUIThreadExecutor executeSyncWithTimeout:block:error:]_block_invoke + 68
50  EarlGrey                            0x000000010ed14b60 __54-[GREYRunLoopSpinner grey_checkConditionInActiveMode:]_block_invoke + 384
51  CoreFoundation                      0x000000010d77525c __CFRUNLOOP_IS_CALLING_OUT_TO_A_BLOCK__ + 12
52  CoreFoundation                      0x000000010d75a304 __CFRunLoopDoBlocks + 356
53  CoreFoundation                      0x000000010d759a75 __CFRunLoopRun + 901
54  CoreFoundation                      0x000000010d759494 CFRunLoopRunSpecific + 420
55  EarlGrey                            0x000000010ed148cf -[GREYRunLoopSpinner grey_checkConditionInActiveMode:] + 399
56  EarlGrey                            0x000000010ed136a7 -[GREYRunLoopSpinner spinWithStopConditionBlock:] + 983
57  EarlGrey                            0x000000010ed17b03 -[GREYUIThreadExecutor executeSyncWithTimeout:block:error:] + 1491
58  EarlGrey                            0x000000010ece9d2a -[GREYElementInteraction performAction:error:] + 2282
59  EarlGrey                            0x000000010ece940f -[GREYElementInteraction performAction:] + 79
60  ios_chrome_ui_egtests               0x0000000104405e5f -[TabHistoryPopupControllerTestCase testTabHistoryMenu] + 8335
61  CoreFoundation                      0x000000010d75705c __invoking___ + 140

Signal caught: Trace/BPT trap: 5
0   EarlGrey                            0x000000010ece72f3 grey_signalHandler + 259
1   libsystem_platform.dylib            0x00000001169bbbba _sigtramp + 26
2   ???                                 0x000000010d67299b 0x0 + 4519831963
3   ios_chrome_ui_egtests               0x00000001044f5fa1 _ZN7logging10LogMessageD2Ev + 4369
4   ios_chrome_ui_egtests               0x00000001044f2bd5 _ZN7logging10LogMessageD1Ev + 21
5   ios_chrome_ui_egtests               0x00000001041f0706 -[Tab goToEntry:] + 582
6   ios_chrome_ui_egtests               0x0000000104031036 -[BrowserViewController navigateToSelectedEntry:] + 406
7   ios_chrome_ui_egtests               0x0000000104035978 -[BrowserViewController chromeExecuteCommand:] + 4392
8   ios_chrome_ui_egtests               0x00000001039f9e08 -[UIResponder(ChromeExecuteCommand) chromeExecuteCommand:] + 104
9   ios_chrome_ui_egtests               0x00000001039f9e08 -[UIResponder(ChromeExecuteCommand) chromeExecuteCommand:] + 104
10  ios_chrome_ui_egtests               0x00000001039f9e08 -[UIResponder(ChromeExecuteCommand) chromeExecuteCommand:] + 104
11  ios_chrome_ui_egtests               0x00000001039f9e08 -[UIResponder(ChromeExecuteCommand) chromeExecuteCommand:] + 104
12  ios_chrome_ui_egtests               0x00000001039f9e08 -[UIResponder(ChromeExecuteCommand) chromeExecuteCommand:] + 104
13  ios_chrome_ui_egtests               0x0000000103dc9583 -[TabHistoryViewController collectionView:didSelectItemAtIndexPath:] + 131
14  UIKit                               0x000000010f7dfdf4 -[UICollectionView _selectItemAtIndexPath:animated:scrollPosition:notifyDelegate:] + 702
15  UIKit                               0x000000010f80a86e -[UICollectionView touchesEnded:withEvent:] + 665
16  UIKit                               0x000000010f0b9c1b forwardTouchMethod + 348
17  UIKit                               0x000000010f0b9ce4 -[UIResponder touchesEnded:withEvent:] + 49
18  UIKit                               0x000000010f0b9c1b forwardTouchMethod + 348
19  UIKit                               0x000000010f0b9ce4 -[UIResponder touchesEnded:withEvent:] + 49
20  UIKit                               0x000000010f3db1dc _UIGestureEnvironmentSortAndSendDelayedTouches + 5645
21  UIKit                               0x000000010f3d5ea3 _UIGestureEnvironmentUpdate + 1472
22  UIKit                               0x000000010f3d589b -[UIGestureEnvironment _deliverEvent:toGestureRecognizers:usingBlock:] + 521
23  UIKit                               0x000000010f3d4a7e -[UIGestureEnvironment _updateGesturesForEvent:window:] + 286
24  UIKit                               0x000000010ef137ad -[UIWindow sendEvent:] + 3989
25  UIKit                               0x000000010eec0a33 -[UIApplication sendEvent:] + 371
26  UIKit                               0x00000001279f6959 -[UIApplicationAccessibility sendEvent:] + 93
27  EarlGrey                            0x000000010ecfad18 -[GREYTouchInjector grey_injectTouches:] + 1496
28  EarlGrey                            0x000000010ecf9f71 -[GREYTouchInjector timerFiredWithZeroToleranceTimer:] + 849
29  EarlGrey                            0x000000010ecfbe95 __50-[GREYZeroToleranceTimer initWithInterval:target:]_block_invoke + 53
30  EarlGrey                            0x000000010ecfc3a7 __78+[GREYZeroToleranceTimer grey_scheduleZeroToleranceTimerWithInterval:handler:]_block_invoke + 39
31  libdispatch.dylib                   0x000000011662112e _dispatch_client_callout + 8
32  libdispatch.dylib                   0x00000001166049d5 _dispatch_continuation_pop + 578
33  libdispatch.dylib                   0x000000011661577d _dispatch_source_latch_and_call + 192
34  libdispatch.dylib                   0x000000011660e5a9 _dispatch_source_invoke + 1259
35  libdispatch.dylib                   0x000000011660833e _dispatch_main_queue_callback_4CF + 559
36  CoreFoundation                      0x000000010d7944f9 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 9
37  CoreFoundation                      0x000000010d759f8d __CFRunLoopRun + 2205
38  CoreFoundation                      0x000000010d759494 CFRunLoopRunSpecific + 420
39  EarlGrey                            0x000000010ed13d7f -[GREYRunLoopSpinner grey_drainRunLoopInActiveModeAndCheckCondition:forTime:] + 687
40  EarlGrey                            0x000000010ed1372d -[GREYRunLoopSpinner spinWithStopConditionBlock:] + 1117
41  EarlGrey                            0x000000010ecf9b4e -[GREYTouchInjector waitUntilAllTouchesAreDeliveredUsingInjector] + 846
42  EarlGrey                            0x000000010ecf8d60 -[GREYSyntheticEvents grey_endTouchesAtPoints:timeElapsedSinceLastTouchDelivery:] + 176
43  EarlGrey                            0x000000010ecf7d0a +[GREYSyntheticEvents touchAlongMultiplePaths:relativeToWindow:forDuration:expendable:] + 1098
44  EarlGrey                            0x000000010ecf786b +[GREYSyntheticEvents touchAlongPath:relativeToWindow:forDuration:expendable:] + 219
45  EarlGrey                            0x000000010ecb973f +[GREYTapper tapOnWindow:numberOfTaps:location:error:] + 399
46  EarlGrey                            0x000000010ecb955d +[GREYTapper tapOnElement:numberOfTaps:location:error:] + 909
47  EarlGrey                            0x000000010ecb87a3 -[GREYTapAction perform:error:] + 371
48  EarlGrey                            0x000000010eceaae0 __46-[GREYElementInteraction performAction:error:]_block_invoke + 1056
49  EarlGrey                            0x000000010ed17eb4 __59-[GREYUIThreadExecutor executeSyncWithTimeout:block:error:]_block_invoke + 68
50  EarlGrey                            0x000000010ed14b60 __54-[GREYRunLoopSpinner grey_checkConditionInActiveMode:]_block_invoke + 384
51  CoreFoundation                      0x000000010d77525c __CFRUNLOOP_IS_CALLING_OUT_TO_A_BLOCK__ + 12
52  CoreFoundation                      0x000000010d75a304 __CFRunLoopDoBlocks + 356
53  CoreFoundation                      0x000000010d759a75 __CFRunLoopRun + 901
54  CoreFoundation                      0x000000010d759494 CFRunLoopRunSpecific + 420
55  EarlGrey                            0x000000010ed148cf -[GREYRunLoopSpinner grey_checkConditionInActiveMode:] + 399
56  EarlGrey                            0x000000010ed136a7 -[GREYRunLoopSpinner spinWithStopConditionBlock:] + 983
57  EarlGrey                            0x000000010ed17b03 -[GREYUIThreadExecutor executeSyncWithTimeout:block:error:] + 1491
58  EarlGrey                            0x000000010ece9d2a -[GREYElementInteraction performAction:error:] + 2282
59  EarlGrey                            0x000000010ece940f -[GREYElementInteraction performAction:] + 79
60  ios_chrome_ui_egtests               0x0000000104405e5f -[TabHistoryPopupControllerTestCase testTabHistoryMenu] + 8335
61  CoreFoundation                      0x000000010d75705c __invoking___ + 140
62  CoreFoundation                      0x000000010d756ee1 -[NSInvocation invoke] + 289
63  EarlGrey                            0x000000010ece04a8 -[GREYTestCaseInvocation invoke] + 104
64  XCTest                              0x000000010ea37f77 __24-[XCTestCase invokeTest]_block_invoke_2 + 481
65  XCTest                              0x000000010ea707df -[XCTestContext performInScope:] + 190
66  XCTest                              0x000000010ea37d83 -[XCTestCase invokeTest] + 255
67  EarlGrey                            0x000000010ecd079e -[XCTestCase(GREYAdditions) grey_invokeTest] + 1230
68  XCTest                              0x000000010ea3859c -[XCTestCase performTest:] + 457
69  XCTest                              0x000000010ea35664 -[XCTestSuite performTest:] + 491
70  XCTest                              0x000000010ea35664 -[XCTestSuite performTest:] + 491
71  XCTest                              0x000000010ea35664 -[XCTestSuite performTest:] + 491
72  XCTest                              0x000000010ea21618 __25-[XCTestDriver _runSuite]_block_invoke + 51
73  XCTest                              0x000000010ea42d2b -[XCTestObservationCenter _observeTestExecutionForBlock:] + 602
74  XCTest                              0x000000010ea214b5 -[XCTestDriver _runSuite] + 436
75  XCTest                              0x000000010ea22302 -[XCTestDriver _checkForTestManager] + 287
76  XCTest                              0x000000010ea71d67 _XCTestMain + 628
77  CoreFoundation                      0x000000010d77525c __CFRUNLOOP_IS_CALLING_OUT_TO_A_BLOCK__ + 12
78  CoreFoundation                      0x000000010d75a304 __CFRunLoopDoBlocks + 356
79  CoreFoundation                      0x000000010d759a75 __CFRunLoopRun + 901
80  CoreFoundation                      0x000000010d759494 CFRunLoopRunSpecific + 420
81  GraphicsServices                    0x0000000114199a6f GSEventRunModal + 161
82  UIKit                               0x000000010eea2f34 UIApplicationMain + 159
83  ios_chrome_ui_egtests               0x000000010346d21b main + 1051
84  libdyld.dylib                       0x000000011666a68d start + 1
2017-02-13 02:23:13.420 xcodebuild[19237:179360] [MT] IDETestOperationsObserverDebug: (7A56559D-C793-4712-9A9F-57BCD927D8CD) Beginning test session ios_chrome_ui_egtests_module-7A56559D-C793-4712-9A9F-57BCD927D8CD at 2017-02-13 02:23:13.420 with Xcode 8A218a on target <DVTiPhoneSimulator: 0x7ffa3cde45f0> {
		SimDevice: SimDevice : iPhone 6s (15437930-52AE-4CA5-93FF-FF908611582D) : state={ Booted } deviceType={ SimDeviceType : com.apple.CoreSimulator.SimDeviceType.iPhone-6s } runtime={ SimRuntime : 10.0 (14A345) - com.apple.CoreSimulator.SimRuntime.iOS-10-0 }
} (10.0 (14A345))
Cc: -kkhorimoto@chromium.org eugene...@chromium.org
Components: UI>Browser>Navigation
Labels: -Pri-2 Pri-1
Owner: kkhorimoto@chromium.org
isEqual: should probably do more, than just comparing pointers.
but more importantly if isEqual: is overridden then hash should also be overridden.
Sorry for breaking this! I'm in the process of removing CRWSessionEntry; I'll prioritize rewriting Tab's |-goToEntry:| to use NavigationItems, which will eliminate the need for the equality test. 
Project Member

Comment 6 by bugdroid1@chromium.org, Feb 13 2017

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

commit 2b55257be75ef0790299a2a239fbb8d33ec4e805
Author: kkhorimoto <kkhorimoto@chromium.org>
Date: Mon Feb 13 23:52:24 2017

Revert "Updated ownership of NavigationItems within CRWSessionController."

The original CL updated behavior in a couple ways that were introducing many bugs:
- New CRWSessionEntries were vended every time a session entry property was
  accessed, which which broke logic comparing CRWSessionEntries.
- CRWSessionEntries held an unowned pointer to their corresponding
  NavigationItems, making it possible for the CRWSessionEntry to outlive its backing
  NavigationItem.

The CL will be re-landed once callers have been updated to reflect this new
behavior.

This reverts 3 CLs:
https://codereview.chromium.org/2672953005 (original CL)
https://codereview.chromium.org/2690973002 (hacky fix)
https://codereview.chromium.org/2680313004 (hacky fix)

BUG=689358, 691634, 545227,  691492 ,  691469 

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

[modify] https://crrev.com/2b55257be75ef0790299a2a239fbb8d33ec4e805/ios/chrome/browser/ui/history/tab_history_popup_controller_unittest.mm
[modify] https://crrev.com/2b55257be75ef0790299a2a239fbb8d33ec4e805/ios/web/BUILD.gn
[modify] https://crrev.com/2b55257be75ef0790299a2a239fbb8d33ec4e805/ios/web/navigation/crw_session_controller.h
[modify] https://crrev.com/2b55257be75ef0790299a2a239fbb8d33ec4e805/ios/web/navigation/crw_session_controller.mm
[modify] https://crrev.com/2b55257be75ef0790299a2a239fbb8d33ec4e805/ios/web/navigation/crw_session_controller_unittest.mm
[modify] https://crrev.com/2b55257be75ef0790299a2a239fbb8d33ec4e805/ios/web/navigation/crw_session_entry.h
[modify] https://crrev.com/2b55257be75ef0790299a2a239fbb8d33ec4e805/ios/web/navigation/crw_session_entry.mm
[modify] https://crrev.com/2b55257be75ef0790299a2a239fbb8d33ec4e805/ios/web/navigation/crw_session_entry_unittest.mm
[modify] https://crrev.com/2b55257be75ef0790299a2a239fbb8d33ec4e805/ios/web/navigation/navigation_item_impl_list.h
[modify] https://crrev.com/2b55257be75ef0790299a2a239fbb8d33ec4e805/ios/web/navigation/navigation_item_impl_list.mm
[modify] https://crrev.com/2b55257be75ef0790299a2a239fbb8d33ec4e805/ios/web/navigation/session_storage_builder.mm
[modify] https://crrev.com/2b55257be75ef0790299a2a239fbb8d33ec4e805/ios/web/public/navigation_item_list.h
[add] https://crrev.com/2b55257be75ef0790299a2a239fbb8d33ec4e805/ios/web/public/navigation_item_list.mm
[modify] https://crrev.com/2b55257be75ef0790299a2a239fbb8d33ec4e805/ios/web/web_state/ui/crw_web_controller.mm
[modify] https://crrev.com/2b55257be75ef0790299a2a239fbb8d33ec4e805/ios/web/web_state/ui/crw_web_controller_unittest.mm

Status: Fixed (was: Assigned)

Sign in to add a comment