Commit 402571f8 authored by Findit's avatar Findit Committed by Noel Gordon

Revert "Files app: Select My files when there are no volumes"

This reverts commit 5cd8c123.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 599026 as the
culprit for flakes in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vNWNkOGMxMjNlNDUyYzBiNzZmYmJjYWY1M2U4MmQ4NmNhN2Q1NDkzMAw

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.memory/Linux%20ChromiumOS%20MSan%20Tests/9016

Sample Failed Step: viz_browser_tests

Sample Flaky Test: FileDisplay/FilesAppBrowserTest.Test/fileDisplayWithoutVolumesThenMountDrive_DriveFs

Original change's description:
> Files app: Select My files when there are no volumes
> 
> Make Files app select "My files" when there are no available volumes,
> this to allow Files app to behave properly when volumes subsequently
> become available.
> 
> Change DirectoryModel.onVolumeInfoListUpdated_ method to check for
> non-null |displayRoot| before trying to change to |displayRoot|. This
> fixes the error "Cannot read property 'getParent' of null" when Drive
> volume becomes available before Downloads volume, which is the default
> volume/root.
> 
> Change FakeDriveFs to unbind the two mojo bindings if they're bound, so
> DriveFsTestVolume can re-mount itself.
> 
> Test: browser_tests --gtest_filter="*/fileDisplayWithoutVolumesThenMount*"
> Bug: 893161, 884967
> Change-Id: Ic813b25261530495c11c9f641a92f6e07f883702
> Reviewed-on: https://chromium-review.googlesource.com/c/1272418
> Reviewed-by: Noel Gordon <noel@chromium.org>
> Reviewed-by: Sam McNally <sammc@chromium.org>
> Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#599026}

Change-Id: I883485c8fc1bdcc22dba93cc4b03b7c157dcb5f2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 893161, 884967, 889703
Reviewed-on: https://chromium-review.googlesource.com/c/1278063
Cr-Commit-Position: refs/heads/master@{#599190}
parent 863a437d
...@@ -197,9 +197,6 @@ WRAPPED_INSTANTIATE_TEST_CASE_P( ...@@ -197,9 +197,6 @@ WRAPPED_INSTANTIATE_TEST_CASE_P(
TestCase("fileSearch"), TestCase("fileSearch"),
TestCase("fileDisplayWithoutDownloadsVolume"), TestCase("fileDisplayWithoutDownloadsVolume"),
TestCase("fileDisplayWithoutVolumes"), TestCase("fileDisplayWithoutVolumes"),
TestCase("fileDisplayWithoutVolumesThenMountDownloads"),
TestCase("fileDisplayWithoutVolumesThenMountDrive"),
TestCase("fileDisplayWithoutVolumesThenMountDrive").EnableDriveFs(),
TestCase("fileSearchCaseInsensitive"), TestCase("fileSearchCaseInsensitive"),
TestCase("fileSearchNotFound"))); TestCase("fileSearchNotFound")));
......
...@@ -828,18 +828,6 @@ class DriveTestVolume : public TestVolume { ...@@ -828,18 +828,6 @@ class DriveTestVolume : public TestVolume {
return integration_service_; return integration_service_;
} }
bool Mount(Profile* profile) {
if (profile != profile_)
return false;
if (!integration_service_)
return false;
integration_service_->SetEnabled(true);
CreateDriveFsConnectionDelegate();
return true;
}
void Unmount() { integration_service_->SetEnabled(false); } void Unmount() { integration_service_->SetEnabled(false); }
private: private:
...@@ -1367,11 +1355,6 @@ void FileManagerBrowserTestBase::OnCommand(const std::string& name, ...@@ -1367,11 +1355,6 @@ void FileManagerBrowserTestBase::OnCommand(const std::string& name,
return; return;
} }
if (name == "mountDrive") {
ASSERT_TRUE(drive_volume_->Mount(profile()));
return;
}
if (name == "mountDownloads") { if (name == "mountDownloads") {
ASSERT_TRUE(local_volume_->Mount(profile())); ASSERT_TRUE(local_volume_->Mount(profile()));
return; return;
......
...@@ -256,8 +256,6 @@ void FakeDriveFs::RegisterMountingForAccountId( ...@@ -256,8 +256,6 @@ void FakeDriveFs::RegisterMountingForAccountId(
std::unique_ptr<drivefs::DriveFsHost::MojoConnectionDelegate> std::unique_ptr<drivefs::DriveFsHost::MojoConnectionDelegate>
FakeDriveFs::CreateConnectionDelegate() { FakeDriveFs::CreateConnectionDelegate() {
drivefs::mojom::DriveFsBootstrapPtrInfo bootstrap; drivefs::mojom::DriveFsBootstrapPtrInfo bootstrap;
if (bootstrap_binding_.is_bound())
bootstrap_binding_.Unbind();
bootstrap_binding_.Bind(mojo::MakeRequest(&bootstrap)); bootstrap_binding_.Bind(mojo::MakeRequest(&bootstrap));
pending_delegate_request_ = mojo::MakeRequest(&delegate_); pending_delegate_request_ = mojo::MakeRequest(&delegate_);
delegate_->OnMounted(); delegate_->OnMounted();
...@@ -289,8 +287,6 @@ void FakeDriveFs::Init(drivefs::mojom::DriveFsConfigurationPtr config, ...@@ -289,8 +287,6 @@ void FakeDriveFs::Init(drivefs::mojom::DriveFsConfigurationPtr config,
drivefs::mojom::DriveFsDelegatePtr delegate) { drivefs::mojom::DriveFsDelegatePtr delegate) {
mojo::FuseInterface(std::move(pending_delegate_request_), mojo::FuseInterface(std::move(pending_delegate_request_),
delegate.PassInterface()); delegate.PassInterface());
if (binding_.is_bound())
binding_.Unbind();
binding_.Bind(std::move(drive_fs_request)); binding_.Bind(std::move(drive_fs_request));
} }
......
...@@ -1202,8 +1202,7 @@ DirectoryModel.prototype.onVolumeInfoListUpdated_ = function(event) { ...@@ -1202,8 +1202,7 @@ DirectoryModel.prototype.onVolumeInfoListUpdated_ = function(event) {
var entry = this.getCurrentDirEntry(); var entry = this.getCurrentDirEntry();
if (entry && !this.volumeManager_.getVolumeInfo(entry)) { if (entry && !this.volumeManager_.getVolumeInfo(entry)) {
this.volumeManager_.getDefaultDisplayRoot(function(displayRoot) { this.volumeManager_.getDefaultDisplayRoot(function(displayRoot) {
if (displayRoot) this.changeDirectoryEntry(displayRoot);
this.changeDirectoryEntry(displayRoot);
}.bind(this)); }.bind(this));
} }
......
...@@ -1426,18 +1426,6 @@ FileManager.prototype = /** @struct */ { ...@@ -1426,18 +1426,6 @@ FileManager.prototype = /** @struct */ {
}.bind(this)); }.bind(this));
}.bind(this)); }.bind(this));
queue.run((callback) => {
// If there is target to be selected, just move to next step.
if (nextCurrentDirEntry) {
callback();
return;
}
// Try to select MyFiles if anything else has failed.
nextCurrentDirEntry = this.directoryTree.dataModel.myFilesModel_.entry;
callback();
});
// Finalize. // Finalize.
queue.run(function(callback) { queue.run(function(callback) {
// Check directory change. // Check directory change.
......
...@@ -392,101 +392,3 @@ testcase.fileDisplayWithoutVolumes = function() { ...@@ -392,101 +392,3 @@ testcase.fileDisplayWithoutVolumes = function() {
}, },
]); ]);
}; };
/**
* Tests Files app opening without errors when there are no volumes at all and
* then mounting Downloads volume which should appear and be able to display its
* files.
*/
testcase.fileDisplayWithoutVolumesThenMountDownloads = function() {
let appId;
StepsRunner.run([
// Unmount all default volumes.
function() {
sendTestMessage({name: 'unmountAllVolumes'}).then(this.next);
},
// Open Files app without specifying the initial directory/root.
function() {
openNewWindow(null, null, this.next);
},
// Wait for Files app to finish loading.
function(result) {
chrome.test.assertTrue(!!result, 'failed to open new window');
appId = result;
remoteCall.waitFor('isFileManagerLoaded', appId, true).then(this.next);
},
// Remount Downloads.
function() {
sendTestMessage({name: 'mountDownloads'}).then(this.next);
},
// Add an entry to Downloads.
function() {
addEntries(['local'], [ENTRIES.newlyAdded], this.next);
},
// Because Downloads is the default volume it will be automatically
// selected, so let's wait for its entry to appear.
function() {
remoteCall.waitForFiles(appId, [ENTRIES.newlyAdded.getExpectedRow()])
.then(this.next);
},
function() {
checkIfNoErrorsOccured(this.next);
},
]);
};
/**
* Tests Files app opening without errors when there are no volumes at all and
* then mounting Drive volume which should appear and be able to display its
* files.
*/
testcase.fileDisplayWithoutVolumesThenMountDrive = function() {
let appId;
const driveTreeItem = '#directory-tree [entry-label="Google Drive"]';
StepsRunner.run([
// Unmount all default volumes.
function() {
sendTestMessage({name: 'unmountAllVolumes'}).then(this.next);
},
// Open Files app without specifying the initial directory/root.
function() {
openNewWindow(null, null, this.next);
},
// Wait for Files app to finish loading.
function(result) {
chrome.test.assertTrue(!!result, 'failed to open new window');
appId = result;
remoteCall.waitFor('isFileManagerLoaded', appId, true).then(this.next);
},
// Remount Drive.
function() {
sendTestMessage({name: 'mountDrive'}).then(this.next);
},
// Add an entry to Drive.
function() {
addEntries(['drive'], [ENTRIES.newlyAdded], this.next);
},
// Wait "Google Drive" to show up in the directory tree.
function() {
remoteCall.waitForElement(appId, driveTreeItem).then(this.next);
},
// Select "My Drive" to display its content.
function() {
const isDriveSubVolume = true;
remoteCall
.callRemoteTestUtil(
'selectInDirectoryTree', appId, [driveTreeItem, isDriveSubVolume])
.then(this.next);
},
// Wait for "My Drive" files to display in the file list.
function() {
remoteCall.waitForFiles(appId, [ENTRIES.newlyAdded.getExpectedRow()])
.then(this.next);
},
function() {
checkIfNoErrorsOccured(this.next);
},
]);
};
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