Commit b162a3d9 authored by Luciano Pacheco's avatar Luciano Pacheco Committed by Commit Bot

Revert "[Reland attempt #2] Refactor DirectoryTree"

This reverts commit b7ecb7e9.

Reason for revert: ChromiumOS MSAN got flaky/failing crbug.com/899664

Original change's description:
> [Reland attempt #2] Refactor DirectoryTree
> 
> Fix flakiness by adding call to this.expanded=true in EntryListItem.
> 
> Change updateSubDirectories:
> - use arrow function and const;
> - change error callback calling style to match the same style used by
> success callback (without if).
> - change check from isFakeEntry to check for presence of the method
> that's used "createReader".
> 
> Change the sorting to be an method, so it can be customized per
> sub-class, later it will be used to sort My files to show Linux and Play
> files at the bottom.
> 
> Fix unittest that started failing because it metadataModel was null.
> 
> No changes in behaviour for users, the small change in behaviour is
> EntryListItem.updateSubDirectories now reads its children using FS API
> which isn't synchronous.
> 
> CrOS FilesApp UI Tests: ensure downloads selected and refreshed in setup
> 
> TBR: joelhockey@chromium.org
> Bug: 899664
> Change-Id: I8a93616f74cf3bf2b99851dc2452249e1353d85e
> Reviewed-on: https://chromium-review.googlesource.com/c/1306963
> Reviewed-by: Joel Hockey <joelhockey@chromium.org>
> Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#604069}

TBR=joelhockey@chromium.org,lucmult@chromium.org

Change-Id: Ic7903e70225a59e3eca7d581ea44202462b01dee
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 899664
Reviewed-on: https://chromium-review.googlesource.com/c/1309570Reviewed-by: default avatarLuciano Pacheco <lucmult@chromium.org>
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604194}
parent a72b9407
...@@ -972,8 +972,6 @@ DirectoryModel.prototype.changeDirectoryEntry = function( ...@@ -972,8 +972,6 @@ DirectoryModel.prototype.changeDirectoryEntry = function(
if (dirEntry instanceof VolumeEntry) if (dirEntry instanceof VolumeEntry)
dirEntry = assert(dirEntry.rootEntry); dirEntry = assert(dirEntry.rootEntry);
// TODO(lucmult): Remove this log once flakiness is fixed.
console.log('changeDirectoryEntry: ', dirEntry.name);
// Increment the sequence value. // Increment the sequence value.
this.changeDirectorySequence_++; this.changeDirectorySequence_++;
this.clearSearch_(); this.clearSearch_();
......
...@@ -482,17 +482,6 @@ DirectoryItem.prototype.handleClick = function(e) { ...@@ -482,17 +482,6 @@ DirectoryItem.prototype.handleClick = function(e) {
} }
}; };
/**
* Default sorting for DirectoryItem sub-dirrectories.
* @param {!Array<!Entry>} entries Entries to be sorted.
* @returns {!Array<!Entry>}
*/
DirectoryItem.prototype.sortEntries = function(entries) {
entries.sort(util.compareName);
const filter = this.fileFilter_.filter.bind(this.fileFilter_);
return entries.filter(filter);
};
/** /**
* Retrieves the latest subdirectories and update them on the tree. * Retrieves the latest subdirectories and update them on the tree.
* @param {boolean} recursive True if the update is recursively. * @param {boolean} recursive True if the update is recursively.
...@@ -501,11 +490,17 @@ DirectoryItem.prototype.sortEntries = function(entries) { ...@@ -501,11 +490,17 @@ DirectoryItem.prototype.sortEntries = function(entries) {
*/ */
DirectoryItem.prototype.updateSubDirectories = function( DirectoryItem.prototype.updateSubDirectories = function(
recursive, opt_successCallback, opt_errorCallback) { recursive, opt_successCallback, opt_errorCallback) {
if (!this.entry || this.entry.createReader === undefined) { if (!this.entry || util.isFakeEntry(this.entry)) {
opt_errorCallback && opt_errorCallback(); if (opt_errorCallback)
opt_errorCallback();
return; return;
} }
const sortEntries = (fileFilter, entries) => {
entries.sort(util.compareName);
return entries.filter(fileFilter.filter.bind(fileFilter));
};
const onSuccess = (entries) => { const onSuccess = (entries) => {
this.entries_ = entries; this.entries_ = entries;
this.updateSubElementsFromList(recursive); this.updateSubElementsFromList(recursive);
...@@ -517,7 +512,7 @@ DirectoryItem.prototype.updateSubDirectories = function( ...@@ -517,7 +512,7 @@ DirectoryItem.prototype.updateSubDirectories = function(
const readEntry = () => { const readEntry = () => {
reader.readEntries((results) => { reader.readEntries((results) => {
if (!results.length) { if (!results.length) {
onSuccess(this.sortEntries(entries)); onSuccess(sortEntries(this.fileFilter_, entries));
return; return;
} }
...@@ -748,46 +743,26 @@ EntryListItem.prototype = { ...@@ -748,46 +743,26 @@ EntryListItem.prototype = {
*/ */
EntryListItem.prototype.updateSubDirectories = function( EntryListItem.prototype.updateSubDirectories = function(
recursive, opt_successCallback, opt_errorCallback) { recursive, opt_successCallback, opt_errorCallback) {
if (!this.entry || this.entry.createReader === undefined) { if (!this.entry) {
opt_errorCallback && opt_errorCallback(); opt_errorCallback && opt_errorCallback();
return; return;
} }
this.entries_ = []; this.entries_ = [];
if (this.entry && this.entry.children) {
const onSuccess = (entries) => { for (let childEntry of this.entry.children) {
this.entries_ = entries; if (childEntry instanceof VolumeEntry) {
this.updateSubElementsFromList(recursive); // For VolumeEntry we want to display its root.
if (this.entries_.length > 0) this.entries_.push(childEntry.rootEntry);
this.expanded = true; } else {
opt_successCallback && opt_successCallback(); this.entries_.push(childEntry);
// TODO(lucmult): Remove this log once flakiness is fixed.
console.log('EntryListItem children loaded.');
};
const reader = this.entry.createReader();
const entries = [];
const readEntry = () => {
reader.readEntries((results) => {
if (!results.length) {
onSuccess(this.sortEntries(entries));
return;
}
for (let i = 0; i < results.length; i++) {
const entry = results[i];
if (entry.isDirectory) {
// For VolumeEntry we want to display its root.
if (entry instanceof VolumeEntry) {
entries.push(entry.rootEntry);
} else {
entries.push(entry);
}
}
} }
readEntry(); }
}); }
}; if (this.entries_.length > 0) {
readEntry(); this.expanded = true;
}
this.updateSubElementsFromList(recursive);
opt_successCallback && opt_successCallback();
}; };
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
......
...@@ -501,9 +501,8 @@ function testUpdateSubElementsFromListSections() { ...@@ -501,9 +501,8 @@ function testUpdateSubElementsFromListSections() {
assertEquals(NavigationSection.MY_FILES, myFilesItem.section); assertEquals(NavigationSection.MY_FILES, myFilesItem.section);
assertEquals(NavigationSection.CLOUD, driveItem.section); assertEquals(NavigationSection.CLOUD, driveItem.section);
const metadataModel = mockMetadataModel();
DirectoryTree.decorate(directoryTree, directoryModel, volumeManager, DirectoryTree.decorate(directoryTree, directoryModel, volumeManager,
metadataModel, fileOperationManager, true); null, fileOperationManager, true);
directoryTree.dataModel = treeModel; directoryTree.dataModel = treeModel;
directoryTree.updateSubElementsFromList(false); directoryTree.updateSubElementsFromList(false);
...@@ -959,10 +958,9 @@ function testInsideMyDriveAndInsideDrive(callback) { ...@@ -959,10 +958,9 @@ function testInsideMyDriveAndInsideDrive(callback) {
.webkitResolveLocalFileSystemURLEntries['filesystem:downloads/folder1'] = .webkitResolveLocalFileSystemURLEntries['filesystem:downloads/folder1'] =
new MockDirectoryEntry(downloadsFileSystem, '/folder1'); new MockDirectoryEntry(downloadsFileSystem, '/folder1');
const metadataModel = mockMetadataModel();
DirectoryTree.decorate( DirectoryTree.decorate(
directoryTree, directoryModel, volumeManager, metadataModel, directoryTree, directoryModel, volumeManager, null, fileOperationManager,
fileOperationManager, true); true);
directoryTree.dataModel = new MockNavigationListModel(volumeManager); directoryTree.dataModel = new MockNavigationListModel(volumeManager);
directoryTree.redraw(true); directoryTree.redraw(true);
......
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