Skip to content

Commit

Permalink
Revert "Files F2: Enable Tasks from the Store."
Browse files Browse the repository at this point in the history 
This reverts commit 59f4088.

Reason for revert: https://crrev.com/c/4150614, which this CL was based on, broke several Crostini tast tests according to the bisector.
   
Bug: b/272557385

Original change's description:
> Files F2: Enable Tasks from the Store.
>
> Change the unittests to work based on the tasks from the store.
>
> Bug: b:228112698
> Change-Id: Ic414215ef389b1611b3b5a62e7e13677b002094d
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4311811
> Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
> Reviewed-by: Wenbo Jie <wenbojie@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1114870}

Bug: b:228112698
Change-Id: Ib473219a261e2bcb16d119652be9f15d2882a3c6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4327613
Owners-Override: Colin Kincaid <ckincaid@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Colin Kincaid <ckincaid@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1115829}
  • Loading branch information
Colin Kincaid authored and Chromium LUCI CQ committed Mar 10, 2023
1 parent 7b8c323 commit 8e743c8
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 177 deletions.
2 changes: 1 addition & 1 deletion ui/file_manager/file_manager/externs/ts/store.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export class Store {
subscribe(observer) {}

/** @param {!StoreObserver} observer */
unsubscribe(observer) {}
usubscribe(observer) {}

/** @return {!State} */
getState() {}
Expand Down
20 changes: 1 addition & 19 deletions ui/file_manager/file_manager/foreground/js/fake_file_selection_handler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@

import {NativeEventTarget as EventTarget} from 'chrome://resources/ash/common/event_target.js';

import {Store} from '../../externs/ts/store.js';
import {updateDirectoryContent, updateSelection} from '../../state/actions/current_directory.js';

import {FileSelection, FileSelectionHandler} from './file_selection.js';

/**
Expand All @@ -22,12 +19,7 @@ export class FakeFileSelectionHandler {

computeAdditionalCallback() {}

/**
* @param entries {!Array<Entry|FilesAppEntry>}
* @param mimeTypes {!Array<string>}
* @param store {Store=}
*/
updateSelection(entries, mimeTypes, store = null) {
updateSelection(entries, mimeTypes) {
this.selection = /** @type {!FileSelection} */ ({
entries: entries,
mimeTypes: mimeTypes,
Expand All @@ -38,16 +30,6 @@ export class FakeFileSelectionHandler {
});
},
});

if (store) {
// Make sure that the entry is in the directory content.
store.dispatch(updateDirectoryContent({entries}));
// Mark the entry as selected.
store.dispatch(updateSelection({
selectedKeys: entries.map(e => e.toURL()),
entries,
}));
}
}

addEventListener(...args) {
Expand Down
58 changes: 20 additions & 38 deletions ui/file_manager/file_manager/foreground/js/task_controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,6 @@ interface ExtractingTasks {
params: chrome.fileManagerPrivate.IOTaskParams;
}

/**
* Small helper function that makes easier to flip from Store to non-Store
* tasks.
*/
function shouldUseStore() {
// Enabling the Store by default, meaning when the experimental flag is ON we
// fallback to the legacy version.
return !util.isFilesAppExperimental();
}

export class TaskController {
private fileTransferController_: FileTransferController|null = null;
private taskHistory_: TaskHistory;
Expand All @@ -75,7 +65,7 @@ export class TaskController {
private lastSelectedEntries_: Entry[];
private store_: Store;
private selectionFilesData_: FileData[] = [];
private selectionKeys_: FileKey[] = [];
private selectionKeys_: FileKey[]|undefined = [];
private selectionTasks_: StoreFileTasks|undefined;

constructor(
Expand All @@ -94,7 +84,7 @@ export class TaskController {
this.lastSelectedEntries_ = [];
this.store_ = getStore();

if (shouldUseStore()) {
if (util.isFilesAppExperimental()) {
this.store_.subscribe(this);
} else {
// These events are superseded by the store.
Expand All @@ -118,20 +108,16 @@ export class TaskController {
}

onStateChanged(newState: State) {
const keys = newState.currentDirectory?.selection.keys ?? [];
const keys = newState.currentDirectory?.selection.keys;
const tasks = newState.currentDirectory?.selection.fileTasks;
if (keys !== this.selectionKeys_ &&
(keys.length > 0 || this.selectionKeys_.length > 0)) {
if (keys !== this.selectionKeys_) {
this.selectionKeys_ = keys;
this.selectionFilesData_ = getFilesData(newState, keys ?? []);
// Kickoff the async/ActionsProducer to fetch the tasks for the new
// selection.
if (shouldUseStore()) {
if (util.isFilesAppExperimental()) {
this.tasks_ = null;
// Only fetch if there is anything to fetch.
if (keys.length > 0) {
this.store_.dispatch(fetchFileTasks(this.selectionFilesData_));
}
this.store_.dispatch(fetchFileTasks(this.selectionFilesData_));
// Hides the button while fetching the tasks.
this.maybeHideButton();
}
Expand Down Expand Up @@ -205,8 +191,7 @@ export class TaskController {
format = extensions[0]!;
}

// Change default was clicked. We should open "change default"
// dialog.
// Change default was clicked. We should open "change default" dialog.
tasks.showTaskPicker(
this.ui_.defaultTaskPicker, str('CHANGE_DEFAULT_MENU_ITEM'),
strf('CHANGE_DEFAULT_CAPTION', format),
Expand Down Expand Up @@ -262,8 +247,8 @@ export class TaskController {

/**
* Populate the #tasks-menu with the open-with tasks. The menu is managed by
* the top task menu Open combobutton, but it is also used as the
* right-click open-with context menu.
* the top task menu Open combobutton, but it is also used as the right-click
* open-with context menu.
*/
private updateTasksDropdown_(fileTasks: FileTasks) {
const combobutton = this.ui_.taskMenuButton;
Expand Down Expand Up @@ -305,8 +290,7 @@ export class TaskController {
// default is not set by policy, we show an item to change default task.
if (defaultTask && !fileTasks.getPolicyDefaultHandlerStatus()) {
combobutton.addSeparator();
// TODO(greengrape): Ensure that the passed object is a
// `DropdownItem`.
// TODO(greengrape): Ensure that the passed object is a `DropdownItem`.
const changeDefaultMenuItem = combobutton.addDropDownItem({
type: TaskMenuItemType.CHANGE_DEFAULT_TASK,
label: str('CHANGE_DEFAULT_MENU_ITEM'),
Expand All @@ -319,8 +303,7 @@ export class TaskController {
}

/**
* Creates sorted array of available task descriptions such as title and
* icon.
* Creates sorted array of available task descriptions such as title and icon.
*
* @param fileTasks File Tasks to create items.
* @return Created array can be used to feed combobox, menus and so on.
Expand All @@ -335,8 +318,7 @@ export class TaskController {
const title = task.title + ' ' + str('DEFAULT_TASK_LABEL');
items.push(createDropdownItem(
task, title, /*bold=*/ true, /*isDefault=*/ true,
/*isPolicyDefault=*/
!!fileTasks.getPolicyDefaultHandlerStatus()));
/*isPolicyDefault=*/ !!fileTasks.getPolicyDefaultHandlerStatus()));
} else {
items.push(createDropdownItem(task));
}
Expand Down Expand Up @@ -395,9 +377,9 @@ export class TaskController {
}

/**
* Get MIME type for an entry. This method first tries to obtain the MIME
* type from metadata. If it fails, this falls back to obtain the MIME type
* from its content or name.
* Get MIME type for an entry. This method first tries to obtain the MIME type
* from metadata. If it fails, this falls back to obtain the MIME type from
* its content or name.
* @param entry An entry to obtain its mime type.
*/
private async getMimeType_(entry: Entry): Promise<string> {
Expand Down Expand Up @@ -434,12 +416,12 @@ export class TaskController {
}

/**
* Explicitly removes the cached tasks first and and re-calculates the
* current tasks.
* Explicitly removes the cached tasks first and and re-calculates the current
* tasks.
*/
private clearCacheAndUpdateTasks_() {
this.tasks_ = null;
if (shouldUseStore()) {
if (util.isFilesAppExperimental()) {
// Dispatch an empty fetch to invalidate any ongoing fetch.
this.store_.dispatch(fetchFileTasks([]));
}
Expand All @@ -449,7 +431,7 @@ export class TaskController {
private maybeHideButton(): boolean {
const selection = this.selectionHandler_.selection;
// For the Store version the other conditions are checked in the store.
const shouldDisableTasks = shouldUseStore() ?
const shouldDisableTasks = util.isFilesAppExperimental() ?
(this.selectionTasks_?.tasks ?? []).length === 0 :
(
// File Picker/Save As doesn't show the "Open" button.
Expand Down Expand Up @@ -501,7 +483,7 @@ export class TaskController {
}

async getFileTasks(): Promise<FileTasks> {
if (shouldUseStore()) {
if (util.isFilesAppExperimental()) {
return this.getFileTasksStore_();
}
const selection = this.selectionHandler_.selection;
Expand Down

0 comments on commit 8e743c8

Please sign in to comment.