diff options
author | mtomasz <mtomasz@chromium.org> | 2015-02-18 00:32:39 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-02-18 08:33:16 +0000 |
commit | e006dba01aa320695c9994bb1788212d8d69d41d (patch) | |
tree | e2f2548e55abd4d7925fca37c25a02771ce9291a /ui | |
parent | e532bf7c003ca73c46ef998c2d464dd1c55312dd (diff) | |
download | chromium_src-e006dba01aa320695c9994bb1788212d8d69d41d.zip chromium_src-e006dba01aa320695c9994bb1788212d8d69d41d.tar.gz chromium_src-e006dba01aa320695c9994bb1788212d8d69d41d.tar.bz2 |
Remove TryClose and CanClose from Files app.
All apps in Chrome OS are non-persistent, and the background page will be
closed automatically after a period of inactivity.
There is no reason to manually handle closing the background page. It causes
more harm than good, as it may cause lost events (if an event is dispatched
before window.close() is called on the background page).
This especially caused flaky audio player tests, as the final sendMessage()
from the testing extension was ignored, as the background page was manually
closed.
TEST=459479
BUG=browser_tests: *OpenAudioFiles/FileManagerBrowserTest.Test*
Review URL: https://codereview.chromium.org/937703002
Cr-Commit-Position: refs/heads/master@{#316783}
Diffstat (limited to 'ui')
8 files changed, 0 insertions, 104 deletions
diff --git a/ui/file_manager/file_manager/background/js/app_window_wrapper.js b/ui/file_manager/file_manager/background/js/app_window_wrapper.js index de19acf..229fae3 100644 --- a/ui/file_manager/file_manager/background/js/app_window_wrapper.js +++ b/ui/file_manager/file_manager/background/js/app_window_wrapper.js @@ -224,8 +224,6 @@ AppWindowWrapper.prototype.onClosed_ = function() { // Remove the window from the set. delete window.background.appWindows[this.id_]; - - window.background.tryClose(); }; /** diff --git a/ui/file_manager/file_manager/background/js/background.js b/ui/file_manager/file_manager/background/js/background.js index f4cb5af..d23106d 100644 --- a/ui/file_manager/file_manager/background/js/background.js +++ b/ui/file_manager/file_manager/background/js/background.js @@ -69,9 +69,6 @@ function FileBrowserBackground() { * @type {!DriveSyncHandler} */ this.driveSyncHandler = new DriveSyncHandler(this.progressCenter); - this.driveSyncHandler.addEventListener( - DriveSyncHandler.COMPLETED_EVENT, - function() { this.tryClose(); }.bind(this)); /** * Provides support for scaning media devices as part of Cloud Import. @@ -105,13 +102,6 @@ function FileBrowserBackground() { */ this.stringData = null; - /** - * Last time when the background page can close. - * - * @private {?number} - */ - this.lastTimeCanClose_ = null; - // Initialize handlers. chrome.fileBrowserHandler.onExecute.addListener(this.onExecute_.bind(this)); chrome.app.runtime.onLaunched.addListener(this.onLaunched_.bind(this)); @@ -129,14 +119,6 @@ function FileBrowserBackground() { }.bind(this)); } -/** - * A number of delay milliseconds from the first call of tryClose to the actual - * close action. - * @const {number} - * @private - */ -FileBrowserBackground.CLOSE_DELAY_MS_ = 5000; - FileBrowserBackground.prototype.__proto__ = BackgroundBase.prototype; /** @@ -175,47 +157,6 @@ FileBrowserBackground.prototype.ready = function(callback) { }; /** - * Checks the current condition of background page. - * @return {boolean} True if the background page is closable, false if not. - */ -FileBrowserBackground.prototype.canClose = function() { - // If the file operation is going, the background page cannot close. - if ((this.fileOperationManager && - this.fileOperationManager.hasQueuedTasks()) || - this.driveSyncHandler.syncing) { - this.lastTimeCanClose_ = null; - return false; - } - - var views = chrome.extension.getViews(); - var closing = false; - for (var i = 0; i < views.length; i++) { - // If the window that is not the background page itself and it is not - // closing, the background page cannot close. - if (views[i] !== window && !views[i].closing) { - this.lastTimeCanClose_ = null; - return false; - } - closing = closing || views[i].closing; - } - - // If some windows are closing, or the background page can close but could not - // 5 seconds ago, We need more time for sure. - if (closing || - this.lastTimeCanClose_ === null || - (Date.now() - this.lastTimeCanClose_ < - FileBrowserBackground.CLOSE_DELAY_MS_)) { - if (this.lastTimeCanClose_ === null) - this.lastTimeCanClose_ = Date.now(); - setTimeout(this.tryClose.bind(this), FileBrowserBackground.CLOSE_DELAY_MS_); - return false; - } - - // Otherwise we can close the background page. - return true; -}; - -/** * Opens the volume root (or opt directoryPath) in main UI. * * @param {!Event} event An event with the volumeId or diff --git a/ui/file_manager/file_manager/background/js/background_base.js b/ui/file_manager/file_manager/background/js/background_base.js index 6815a77..340cbb6 100644 --- a/ui/file_manager/file_manager/background/js/background_base.js +++ b/ui/file_manager/file_manager/background/js/background_base.js @@ -22,22 +22,6 @@ function BackgroundBase() { } /** - * Checks the current condition of background page. - * @return {boolean} True if the background page can be closed. False if not. - */ -BackgroundBase.prototype.canClose = function() { - return true; -}; - -/** - * Checks the current condition of background page and closes it if possible. - */ -BackgroundBase.prototype.tryClose = function() { - if (this.canClose()) - window.close(); -}; - -/** * Gets similar windows, it means with the same initial url. * @param {string} url URL that the obtained windows have. * @return {Array.<chrome.app.window.AppWindow>} List of similar windows. diff --git a/ui/file_manager/file_manager/background/js/file_operation_handler.js b/ui/file_manager/file_manager/background/js/file_operation_handler.js index d5730fc..b7126e2 100644 --- a/ui/file_manager/file_manager/background/js/file_operation_handler.js +++ b/ui/file_manager/file_manager/background/js/file_operation_handler.js @@ -163,9 +163,6 @@ FileOperationHandler.getType_ = function(operationType) { FileOperationHandler.prototype.onCopyProgress_ = function(event) { var EventType = fileOperationUtil.EventRouter.EventType; event = /** @type {FileOperationProgressEvent} */ (event); - // If the copy is finished, may be we can close the background page. - if (event.reason !== EventType.BEGIN && event.reason !== EventType.PROGRESS) - this.background_.tryClose(); // Update progress center. var progressCenter = this.progressCenter_; @@ -231,9 +228,6 @@ FileOperationHandler.prototype.onCopyProgress_ = function(event) { FileOperationHandler.prototype.onDeleteProgress_ = function(event) { var EventType = fileOperationUtil.EventRouter.EventType; event = /** @type {FileOperationProgressEvent} */ (event); - // If the copy is finished, may be we can close the background page. - if (event.reason !== EventType.BEGIN && event.reason !== EventType.PROGRESS) - this.background_.tryClose(); // Update progress center. var progressCenter = this.progressCenter_; diff --git a/ui/file_manager/file_manager/background/js/file_operation_handler_unittest.js b/ui/file_manager/file_manager/background/js/file_operation_handler_unittest.js index 93033d4..317efeb 100644 --- a/ui/file_manager/file_manager/background/js/file_operation_handler_unittest.js +++ b/ui/file_manager/file_manager/background/js/file_operation_handler_unittest.js @@ -68,7 +68,6 @@ function testCopySuccess() { assertEquals('copy', item.type); assertEquals(true, item.single); assertEquals(100, item.progressRateInPercent); - assertEquals(1, background.closeRequestCount); } // Test for copy cancel. @@ -113,7 +112,6 @@ function testCopyCancel() { assertEquals('copy', item.type); assertEquals(true, item.single); assertEquals(0, item.progressRateInPercent); - assertEquals(1, background.closeRequestCount); } // Test for copy target exists error. @@ -139,7 +137,6 @@ function testCopyTargetExistsError() { assertEquals('copy', item.type); assertEquals(true, item.single); assertEquals(0, item.progressRateInPercent); - assertEquals(1, background.closeRequestCount); } // Test for copy file system error. @@ -165,7 +162,6 @@ function testCopyFileSystemError() { assertEquals('copy', item.type); assertEquals(true, item.single); assertEquals(0, item.progressRateInPercent); - assertEquals(1, background.closeRequestCount); } // Test for copy unexpected error. @@ -191,5 +187,4 @@ function testCopyUnexpectedError() { assertEquals('copy', item.type); assertEquals(true, item.single); assertEquals(0, item.progressRateInPercent); - assertEquals(1, background.closeRequestCount); } diff --git a/ui/file_manager/file_manager/background/js/mock_background.js b/ui/file_manager/file_manager/background/js/mock_background.js index c9c605e..eddcf06 100644 --- a/ui/file_manager/file_manager/background/js/mock_background.js +++ b/ui/file_manager/file_manager/background/js/mock_background.js @@ -9,12 +9,4 @@ function MockBackground() { this.fileOperationManager = new MockFileOperationManager(); this.progressCenter = new MockProgressCenter(); - this.closeRequestCount = 0; } - -/** - * Increments the close request counter. - */ -MockBackground.prototype.tryClose = function() { - this.closeRequestCount++; -}; diff --git a/ui/file_manager/file_manager/common/js/externs.js b/ui/file_manager/file_manager/common/js/externs.js index 3de1016..8569f65 100644 --- a/ui/file_manager/file_manager/common/js/externs.js +++ b/ui/file_manager/file_manager/common/js/externs.js @@ -71,8 +71,3 @@ DirectoryChangeEvent.prototype.newDirEntry; /** @type {boolean} */ DirectoryChangeEvent.prototype.volumeChanged; - -/** - * @type {boolean} - */ -Window.prototype.closing; diff --git a/ui/file_manager/file_manager/foreground/js/file_manager.js b/ui/file_manager/file_manager/foreground/js/file_manager.js index 2035e99..0c63022 100644 --- a/ui/file_manager/file_manager/foreground/js/file_manager.js +++ b/ui/file_manager/file_manager/foreground/js/file_manager.js @@ -1267,9 +1267,6 @@ FileManager.prototype = /** @struct */ { } this.backgroundPage_.background.progressCenter.removePanel( this.ui_.progressCenterPanel); - window.closing = true; - if (this.backgroundPage_) - this.backgroundPage_.background.tryClose(); }; /** |