Commit 10306935 authored by Christos Froussios's avatar Christos Froussios Committed by Commit Bot

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

This reverts commit 6f65bd26.

Reason for revert: Speculative revert
Suspected of breaking browser_tests, ebui_polymer2_browser_tests, single_process_mash_browser_tests and viz_browser_tests
on Linux ChromiumOS MSan

Note: resolved conflicts with ES6 refactoring, therefore running tests...

Bug: 896219

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 the (default) Downloads volume/root.
>
> Change FakeDriveFs to unbind the two mojo bindings if they're bound, so
> DriveFsTestVolume can re-mount itself.
>
> Changes since revert:
> 1. Add a new function |getVolumesCount| to return the number of volumes
> available in the background page.
> 2. Add the new function above to wait for background page to have all
> volumes before un-mounting and wait it to unmount all volumes.
> 3. Change some logs from error to warn because errors are expected and
> handled when initializing volumes that are unmounting/unmounted in the
> backend.
> 4. Change selector used to click on Drive to actually wait for
> "My Drive" to be available.
> 5. A bit of more info in some logs around volume initialization.
>
> Test: browser_tests --gtest_filter="*/fileDisplayWithoutVolumesThenMount*"
> Bug: 893161, 884967, 894799
> Change-Id: I7dcb340991750e9e79a9963990b614c6d7275890
> Reviewed-on: https://chromium-review.googlesource.com/c/1278619
> 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@{#600282}

TBR=noel@chromium.org,sammc@chromium.org,lucmult@chromium.org

Change-Id: Ifaadea8b49cffc6deb70b429a42bab39d4a3d52b
Bug: 893161, 884967, 894799
Reviewed-on: https://chromium-review.googlesource.com/c/1286146
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Reviewed-by: default avatarChristos Froussios <cfroussios@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600376}
parent 3f33f818
......@@ -205,9 +205,6 @@ WRAPPED_INSTANTIATE_TEST_CASE_P(
TestCase("fileSearch"),
TestCase("fileDisplayWithoutDownloadsVolume"),
TestCase("fileDisplayWithoutVolumes"),
TestCase("fileDisplayWithoutVolumesThenMountDownloads"),
TestCase("fileDisplayWithoutVolumesThenMountDrive").DisableDriveFs(),
TestCase("fileDisplayWithoutVolumesThenMountDrive").EnableDriveFs(),
TestCase("fileSearchCaseInsensitive"),
TestCase("fileSearchNotFound")));
......
......@@ -828,18 +828,6 @@ class DriveTestVolume : public TestVolume {
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); }
private:
......@@ -1367,11 +1355,6 @@ void FileManagerBrowserTestBase::OnCommand(const std::string& name,
return;
}
if (name == "mountDrive") {
ASSERT_TRUE(drive_volume_->Mount(profile()));
return;
}
if (name == "mountDownloads") {
ASSERT_TRUE(local_volume_->Mount(profile()));
return;
......
......@@ -256,8 +256,6 @@ void FakeDriveFs::RegisterMountingForAccountId(
std::unique_ptr<drivefs::DriveFsHost::MojoConnectionDelegate>
FakeDriveFs::CreateConnectionDelegate() {
drivefs::mojom::DriveFsBootstrapPtrInfo bootstrap;
if (bootstrap_binding_.is_bound())
bootstrap_binding_.Unbind();
bootstrap_binding_.Bind(mojo::MakeRequest(&bootstrap));
pending_delegate_request_ = mojo::MakeRequest(&delegate_);
delegate_->OnMounted();
......@@ -289,8 +287,6 @@ void FakeDriveFs::Init(drivefs::mojom::DriveFsConfigurationPtr config,
drivefs::mojom::DriveFsDelegatePtr delegate) {
mojo::FuseInterface(std::move(pending_delegate_request_),
delegate.PassInterface());
if (binding_.is_bound())
binding_.Unbind();
binding_.Bind(std::move(drive_fs_request));
}
......
......@@ -710,16 +710,3 @@ test.util.sync.isFileManagerLoaded = function(contentWindow) {
return false;
};
/**
* Reports to the given |callback| the number of volumes available in
* VolumeManager in the background page.
*
* @param {function(number)} callback Callback function to be called with the
* number of volumes.
*/
test.util.async.getVolumesCount = function(callback) {
return volumeManagerFactory.getInstance().then((volumeManager) => {
callback(volumeManager.volumeInfoList.length);
});
};
......@@ -129,8 +129,8 @@ VolumeManagerImpl.prototype.initialize_ = function(callback) {
this.onMountCompleted_.bind(this));
console.debug('Requesting volume list.');
chrome.fileManagerPrivate.getVolumeMetadataList(function(volumeMetadataList) {
console.debug(
'Volume list fetched with: ' + volumeMetadataList.length + ' items.');
console.debug('Volume list fetched with: ' + volumeMetadataList.length +
' items.');
// We must subscribe to the mount completed event in the callback of
// getVolumeMetadataList. crbug.com/330061.
// But volumes reported by onMountCompleted events must be added after the
......@@ -215,7 +215,6 @@ VolumeManagerImpl.prototype.onMountCompleted_ = function(event) {
this.finishRequest_(requestKey, status);
if (event.status === 'success')
this.volumeInfoList.remove(event.volumeMetadata.volumeId);
console.debug('unmounted volume: ' + volumeId);
callback();
break;
}
......
......@@ -92,9 +92,7 @@ volumeManagerUtil.createVolumeInfo = function(volumeMetadata) {
break;
}
console.debug(
'Requesting file system: ' + volumeMetadata.volumeType + ' ' +
volumeMetadata.volumeId);
console.debug('Requesting file system.');
return util.timeoutPromise(
new Promise(function(resolve, reject) {
chrome.fileSystem.requestFileSystem(
......@@ -153,8 +151,8 @@ volumeManagerUtil.createVolumeInfo = function(volumeMetadata) {
fileSystem.root.createReader().readEntries(
function() { /* do nothing */ },
function(error) {
console.warn(
'Triggering full feed fetch has failed: ' +
console.error(
'Triggering full feed fetch is failed: ' +
error.name);
});
}
......@@ -176,7 +174,7 @@ volumeManagerUtil.createVolumeInfo = function(volumeMetadata) {
* @param {*} error
*/
function(error) {
console.warn('Failed to mount a file system: ' +
console.error('Failed to mount a file system: ' +
volumeMetadata.volumeId + ' because of: ' +
(error.stack || error));
......
......@@ -1202,8 +1202,7 @@ DirectoryModel.prototype.onVolumeInfoListUpdated_ = function(event) {
const entry = this.getCurrentDirEntry();
if (entry && !this.volumeManager_.getVolumeInfo(entry)) {
this.volumeManager_.getDefaultDisplayRoot((displayRoot) => {
if (displayRoot)
this.changeDirectoryEntry(displayRoot);
this.changeDirectoryEntry(displayRoot);
});
}
......
......@@ -1424,14 +1424,6 @@ FileManager.prototype = /** @struct */ {
});
});
// If there is no target select MyFiles by default.
queue.run((callback) => {
if (!nextCurrentDirEntry)
nextCurrentDirEntry = this.directoryTree.dataModel.myFilesModel_.entry;
callback();
});
// Finalize.
queue.run((callback) => {
// Check directory change.
......
......@@ -343,27 +343,13 @@ testcase.fileSearchNotFound = function() {
* the default volume.
*/
testcase.fileDisplayWithoutDownloadsVolume = function() {
let appId = null;
let appId;
StepsRunner.run([
// Wait for the Files app background page to mount the default volumes.
function() {
const args = [];
// appId is still null, but isn't needed for getVolumesCount.
remoteCall.waitFor('getVolumesCount', appId, (count) => count === 3, args)
.then(this.next);
},
// Unmount Downloads volume which the default volume.
function() {
sendTestMessage({name: 'unmountDownloads'}).then(this.next);
},
// Wait until all volumes are removed.
function() {
const args = [];
// appId is still null, but isn't needed for getVolumesCount.
remoteCall.waitFor('getVolumesCount', appId, (count) => count === 2, args)
.then(this.next);
},
// Open Files app without specifying the initial directory/root.
function() {
openNewWindow(null, null, this.next);
......@@ -384,70 +370,13 @@ testcase.fileDisplayWithoutDownloadsVolume = function() {
* Tests Files app opening without errors when there are no volumes at all.
*/
testcase.fileDisplayWithoutVolumes = function() {
let appId = null;
StepsRunner.run([
// Wait for the Files app background page to mount the default volumes.
function() {
const args = [];
// appId is still null, but isn't needed for getVolumesCount.
remoteCall.waitFor('getVolumesCount', appId, (count) => count === 3, args)
.then(this.next);
},
// Unmount all default volumes.
function() {
sendTestMessage({name: 'unmountAllVolumes'}).then(this.next);
},
// Wait until all volumes are removed.
function() {
const args = [];
// appId is still null, but isn't needed for getVolumesCount.
remoteCall.waitFor('getVolumesCount', appId, (count) => count === 0, args)
.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);
},
function() {
checkIfNoErrorsOccured(this.next);
},
]);
};
/**
* 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 = null;
let appId;
StepsRunner.run([
// Wait for the Files app background page to mount the default volumes.
function() {
const args = [];
// appId is still null, but isn't needed for getVolumesCount.
remoteCall.waitFor('getVolumesCount', appId, (count) => count === 3, args)
.then(this.next);
},
// Unmount all default volumes.
function() {
sendTestMessage({name: 'unmountAllVolumes'}).then(this.next);
},
// Wait until all volumes are removed.
function() {
const args = [];
// appId is still null, but isn't needed for getVolumesCount.
remoteCall.waitFor('getVolumesCount', appId, (count) => count === 0, args)
.then(this.next);
},
// Open Files app without specifying the initial directory/root.
function() {
openNewWindow(null, null, this.next);
......@@ -458,95 +387,6 @@ testcase.fileDisplayWithoutVolumesThenMountDownloads = function() {
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 = null;
// Selector for waiting Drive gran-root containing "My Drive" root, because
// Drive can be displayed before "My Drive" is available and in this case the
// "click" event on Drive grand-root doesn't work.
const driveTreeItem = '#directory-tree [entry-label="Google Drive"] ' +
'.tree-row[has-children="true"] + .tree-children ' +
'.tree-item[entry-label="My Drive"]';
StepsRunner.run([
// Wait for the Files app background page to mount the default volumes.
function() {
const args = [];
// appId is still null, but isn't needed for getVolumesCount.
remoteCall.waitFor('getVolumesCount', appId, (count) => count === 3, args)
.then(this.next);
},
// Unmount all default volumes.
function() {
sendTestMessage({name: 'unmountAllVolumes'}).then(this.next);
},
// Wait until all volumes are removed.
function() {
const args = [];
// appId is still null, but isn't needed for getVolumesCount.
remoteCall.waitFor('getVolumesCount', appId, (count) => count === 0, args)
.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(result) {
chrome.test.assertTrue(result);
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