Commit de17fd16 authored by Mark Cogan's avatar Mark Cogan Committed by Commit Bot

[iOS] Fix test crashes with multiwindow enabled.

This CL fixes a variety of issues that will cause crashes or timeouts
in EG and unit tests with the multiwindow flag enabled:

- InstallationNotifier will keep polling forever by default, and since
  it's a singleton that lives through the whole test app's lifetime, it
  can hit DCHECKs when objects it depends on get eventually deallocated.
  This CL adds a method to stop polling, and uses it in the download
  manager test (which was triggering the polling).

  Long-term, this shouldn't be a singleton.

- SelectorCoordinatorTest needed to have explicit timeouts added to
  some waits; it was spinning forever in the simulator with MW enabled.

- PassphraseTableViewControllerTest needed to create some dummy state
  objects, because one of the settings view controllers grabs the scene
  state. There are several layers of cleaner fixes possible here --
  refactoring the view controller to not access model/app objects, etc.

- The history and bookmarks EG tests include tests for opening new
  windows, but they cause various EG test failures when they are actually
  run with the flag enabled.

With this CL in place, there are still numerous unit test *failures*
with multiwindow enabled, but not any crashes or timeouts.

(Prior versions of this CL enabled multi-window on trunk; it now doesn't
do that).


Change-Id: Idde141baa4f28d17828367fb359630723c2d12dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2310676
Commit-Queue: Mark Cogan <marq@chromium.org>
Reviewed-by: default avatarDavid Jean <djean@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805841}
parent bf10329a
......@@ -33,6 +33,9 @@
// registered observers need to know the accurate state of installed native
// apps.
- (void)checkNow;
// Stops any queued polling.
- (void)stopPolling;
@end
#endif // IOS_CHROME_BROWSER_INSTALLATION_NOTIFIER_H_
......@@ -152,6 +152,13 @@ const net::BackoffEntry::Policy kPollingBackoffPolicy = {
[self pollForTheInstallationOfApps];
}
- (void)stopPolling {
// Increment the queued block ID, making it higher than the block ID of any
// currently queued block, which will prevent them from running (and from
// queueing any new blocks).
++_lastCreatedBlockId;
}
- (void)dispatchInstallationNotifierBlock {
DCHECK_CURRENTLY_ON(web::WebThread::UI);
int blockId = ++_lastCreatedBlockId;
......
......@@ -211,7 +211,9 @@ using chrome_test_util::TappableBookmarkNodeWithLabel;
// Tests display and selection of 'Open in New Window' in a context menu on a
// bookmarks entry.
- (void)testContextMenuOpenInNewWindow {
// TODO(crbug.com/1126893): reenable this test once EG multiwindow support is
// available.
- (void)DISABLED_testContextMenuOpenInNewWindow {
// TODO(crbug.com/1035764): EG1 Test fails on iOS 12.
if (!base::ios::IsRunningOnIOS13OrLater()) {
EARL_GREY_TEST_DISABLED(@"EG1 Fails on iOS 12.");
......
......@@ -21,6 +21,7 @@
#include "ios/chrome/browser/download/download_manager_metric_names.h"
#import "ios/chrome/browser/download/download_manager_tab_helper.h"
#import "ios/chrome/browser/download/external_app_util.h"
#import "ios/chrome/browser/installation_notifier.h"
#include "ios/chrome/browser/main/test_browser.h"
#import "ios/chrome/browser/overlays/public/overlay_request_queue.h"
#import "ios/chrome/browser/overlays/public/web_content_area/alert_overlay.h"
......@@ -102,6 +103,7 @@ class DownloadManagerCoordinatorTest : public PlatformTest {
[document_interaction_controller_class_ stopMocking];
[application_ stopMocking];
[[InstallationNotifier sharedInstance] stopPolling];
}
web::WebTaskEnvironment task_environment_;
......
......@@ -406,7 +406,9 @@ std::unique_ptr<net::test_server::HttpResponse> StandardResponse(
// Tests display and selection of 'Open in New Window' in a context menu on a
// history entry.
- (void)testContextMenuOpenInNewWindow {
// TODO(crbug.com/1126893): reenable this test once EG multiwindow support is
// available.
- (void)DISABLED_testContextMenuOpenInNewWindow {
if (!IsMultipleScenesSupported())
return;
......
......@@ -232,6 +232,7 @@ source_set("test_support") {
"//components/sync_preferences",
"//components/sync_preferences:test_support",
"//google_apis",
"//ios/chrome/app/application_delegate:app_state_header",
"//ios/chrome/browser",
"//ios/chrome/browser/browser_state",
"//ios/chrome/browser/browser_state:test_support",
......@@ -242,6 +243,7 @@ source_set("test_support") {
"//ios/chrome/browser/signin:test_support",
"//ios/chrome/browser/sync",
"//ios/chrome/browser/sync:test_support",
"//ios/chrome/browser/ui/main:scene_state_header",
"//ios/chrome/browser/ui/settings/password",
"//ios/chrome/browser/ui/table_view:test_support",
"//ios/chrome/test/app:test_support",
......
......@@ -22,7 +22,9 @@ namespace web {
class BrowserState;
} // namespace web
@class AppState;
class Browser;
@class SceneState;
class TestChromeBrowserState;
@class UINavigationController;
@class UIViewController;
......@@ -58,6 +60,11 @@ class PassphraseTableViewControllerTest : public ChromeTableViewControllerTest {
// Only valid when SetUpNavigationController has been called.
UIViewController* dummy_controller_;
UINavigationController* nav_controller_;
// Dummy scene state.
SceneState* scene_state_;
// Dummy app state.
AppState* app_state_;
};
#endif // IOS_CHROME_BROWSER_UI_SETTINGS_PASSPHRASE_TABLE_VIEW_CONTROLLER_TEST_H_
......@@ -14,6 +14,7 @@
#include "components/sync/driver/mock_sync_service.h"
#include "components/sync_preferences/pref_service_mock_factory.h"
#include "components/sync_preferences/pref_service_syncable.h"
#import "ios/chrome/app/application_delegate/app_state.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
#include "ios/chrome/browser/main/test_browser.h"
......@@ -23,6 +24,8 @@
#include "ios/chrome/browser/sync/profile_sync_service_factory.h"
#include "ios/chrome/browser/sync/sync_setup_service.h"
#include "ios/chrome/browser/sync/sync_setup_service_factory.h"
#import "ios/chrome/browser/ui/main/scene_state.h"
#import "ios/chrome/browser/ui/main/scene_state_browser_agent.h"
#import "ios/chrome/browser/ui/settings/settings_navigation_controller.h"
#import "ios/public/provider/chrome/browser/signin/fake_chrome_identity_service.h"
#import "testing/gtest_mac.h"
......@@ -76,6 +79,11 @@ void PassphraseTableViewControllerTest::SetUp() {
WebStateList* web_state_list = nullptr;
browser_ = std::make_unique<TestBrowser>(chrome_browser_state_.get(),
web_state_list);
app_state_ = [[AppState alloc] initWithBrowserLauncher:nil
startupInformation:nil
applicationDelegate:nil];
scene_state_ = [[SceneState alloc] initWithAppState:app_state_];
SceneStateBrowserAgent::CreateForBrowser(browser_.get(), scene_state_);
fake_sync_service_ = static_cast<syncer::MockSyncService*>(
ProfileSyncServiceFactory::GetInstance()->SetTestingFactoryAndUse(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment