Merge pull request #4314 from deepak1556/download_cancel_patch
fix crash when default download is canceled
This commit is contained in:
commit
f624800881
6 changed files with 97 additions and 26 deletions
|
@ -12,6 +12,7 @@
|
|||
#include "atom/common/native_mate_converters/gurl_converter.h"
|
||||
#include "atom/common/node_includes.h"
|
||||
#include "base/memory/linked_ptr.h"
|
||||
#include "base/message_loop/message_loop.h"
|
||||
#include "base/strings/utf_string_conversions.h"
|
||||
#include "native_mate/dictionary.h"
|
||||
#include "net/base/filename_util.h"
|
||||
|
@ -70,8 +71,17 @@ DownloadItem::DownloadItem(content::DownloadItem* download_item) :
|
|||
}
|
||||
|
||||
DownloadItem::~DownloadItem() {
|
||||
if (download_item_)
|
||||
OnDownloadDestroyed(download_item_);
|
||||
Destroy(download_item_);
|
||||
}
|
||||
|
||||
void DownloadItem::Destroy(content::DownloadItem* item) {
|
||||
if (item) {
|
||||
OnDownloadDestroyed(item);
|
||||
MarkDestroyed();
|
||||
|
||||
// Destroy the native class in next tick.
|
||||
base::MessageLoop::current()->PostTask(FROM_HERE, GetDestroyClosure());
|
||||
}
|
||||
}
|
||||
|
||||
void DownloadItem::OnDownloadUpdated(content::DownloadItem* item) {
|
||||
|
@ -79,8 +89,8 @@ void DownloadItem::OnDownloadUpdated(content::DownloadItem* item) {
|
|||
}
|
||||
|
||||
void DownloadItem::OnDownloadDestroyed(content::DownloadItem* download_item) {
|
||||
download_item_->RemoveObserver(this);
|
||||
auto iter = g_download_item_objects.find(download_item_->GetId());
|
||||
download_item->RemoveObserver(this);
|
||||
auto iter = g_download_item_objects.find(download_item->GetId());
|
||||
if (iter != g_download_item_objects.end())
|
||||
g_download_item_objects.erase(iter);
|
||||
download_item_ = nullptr;
|
||||
|
|
|
@ -36,6 +36,8 @@ class DownloadItem : public mate::TrackableObject<DownloadItem>,
|
|||
static void BuildPrototype(v8::Isolate* isolate,
|
||||
v8::Local<v8::ObjectTemplate> prototype);
|
||||
|
||||
void Destroy(content::DownloadItem* download_item);
|
||||
|
||||
protected:
|
||||
explicit DownloadItem(content::DownloadItem* download_item);
|
||||
~DownloadItem();
|
||||
|
|
|
@ -306,12 +306,14 @@ void Session::OnDownloadCreated(content::DownloadManager* manager,
|
|||
auto web_contents = item->GetWebContents();
|
||||
if (SavePageHandler::IsSavePageTypes(item->GetMimeType()))
|
||||
return;
|
||||
auto download_item_handle = DownloadItem::Create(isolate(), item);
|
||||
bool prevent_default = Emit(
|
||||
"will-download",
|
||||
DownloadItem::Create(isolate(), item),
|
||||
download_item_handle,
|
||||
api::WebContents::CreateFrom(isolate(), web_contents));
|
||||
if (prevent_default) {
|
||||
item->Cancel(true);
|
||||
download_item_handle->Destroy(item);
|
||||
item->Remove();
|
||||
}
|
||||
}
|
||||
|
|
|
@ -60,7 +60,8 @@ The following events are available on instances of `Session`:
|
|||
|
||||
Emitted when Electron is about to download `item` in `webContents`.
|
||||
|
||||
Calling `event.preventDefault()` will cancel the download.
|
||||
Calling `event.preventDefault()` will cancel the download and `item` will not be
|
||||
available from next tick of the process.
|
||||
|
||||
```javascript
|
||||
session.defaultSession.on('will-download', function(event, item, webContents) {
|
||||
|
|
|
@ -144,6 +144,50 @@ describe('session module', function() {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('session will-download event', function() {
|
||||
var w = null;
|
||||
|
||||
beforeEach(function() {
|
||||
w = new BrowserWindow({
|
||||
show: false,
|
||||
width: 400,
|
||||
height: 400
|
||||
});
|
||||
});
|
||||
afterEach(function() {
|
||||
return w.destroy();
|
||||
});
|
||||
|
||||
it('can cancel default download behavior', function(done) {
|
||||
const mockFile = new Buffer(1024);
|
||||
const contentDisposition = 'inline; filename="mockFile.txt"';
|
||||
const downloadServer = http.createServer(function(req, res) {
|
||||
res.writeHead(200, {
|
||||
'Content-Length': mockFile.length,
|
||||
'Content-Type': 'application/plain',
|
||||
'Content-Disposition': contentDisposition
|
||||
});
|
||||
res.end(mockFile);
|
||||
downloadServer.close();
|
||||
});
|
||||
|
||||
downloadServer.listen(0, '127.0.0.1', function() {
|
||||
const port = downloadServer.address().port;
|
||||
const url = "http://127.0.0.1:" + port + '/';
|
||||
|
||||
ipcRenderer.sendSync('set-download-option', false, true);
|
||||
w.loadURL(url);
|
||||
ipcRenderer.once('download-error', function(event, downloadUrl, filename, error) {
|
||||
assert.equal(downloadUrl, url);
|
||||
assert.equal(filename, 'mockFile.txt');
|
||||
assert.equal(error, 'Object has been destroyed');
|
||||
done();
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
return describe('DownloadItem', function() {
|
||||
var assertDownload, contentDisposition, downloadFilePath, downloadServer, mockPDF;
|
||||
mockPDF = new Buffer(1024 * 1024 * 5);
|
||||
|
@ -173,7 +217,7 @@ describe('session module', function() {
|
|||
return downloadServer.listen(0, '127.0.0.1', function() {
|
||||
var port;
|
||||
port = downloadServer.address().port;
|
||||
ipcRenderer.sendSync('set-download-option', false);
|
||||
ipcRenderer.sendSync('set-download-option', false, false);
|
||||
w.loadURL(url + ":" + port);
|
||||
return ipcRenderer.once('download-done', function(event, state, url, mimeType, receivedBytes, totalBytes, disposition, filename) {
|
||||
assertDownload(event, state, url, mimeType, receivedBytes, totalBytes, disposition, filename, port);
|
||||
|
@ -185,7 +229,7 @@ describe('session module', function() {
|
|||
return downloadServer.listen(0, '127.0.0.1', function() {
|
||||
var port, webview;
|
||||
port = downloadServer.address().port;
|
||||
ipcRenderer.sendSync('set-download-option', false);
|
||||
ipcRenderer.sendSync('set-download-option', false, false);
|
||||
webview = new WebView;
|
||||
webview.src = "file://" + fixtures + "/api/blank.html";
|
||||
webview.addEventListener('did-finish-load', function() {
|
||||
|
@ -199,11 +243,11 @@ describe('session module', function() {
|
|||
return document.body.appendChild(webview);
|
||||
});
|
||||
});
|
||||
return it('can cancel download', function(done) {
|
||||
it('can cancel download', function(done) {
|
||||
return downloadServer.listen(0, '127.0.0.1', function() {
|
||||
var port;
|
||||
port = downloadServer.address().port;
|
||||
ipcRenderer.sendSync('set-download-option', true);
|
||||
ipcRenderer.sendSync('set-download-option', true, false);
|
||||
w.loadURL(url + ":" + port + "/");
|
||||
return ipcRenderer.once('download-done', function(event, state, url, mimeType, receivedBytes, totalBytes, disposition, filename) {
|
||||
assert.equal(state, 'cancelled');
|
||||
|
|
|
@ -102,23 +102,35 @@ app.on('ready', function() {
|
|||
// For session's download test, listen 'will-download' event in browser, and
|
||||
// reply the result to renderer for verifying
|
||||
var downloadFilePath = path.join(__dirname, '..', 'fixtures', 'mock.pdf');
|
||||
ipcMain.on('set-download-option', function(event, need_cancel) {
|
||||
window.webContents.session.once('will-download',
|
||||
function(e, item, webContents) {
|
||||
item.setSavePath(downloadFilePath);
|
||||
item.on('done', function(e, state) {
|
||||
window.webContents.send('download-done',
|
||||
state,
|
||||
item.getURL(),
|
||||
item.getMimeType(),
|
||||
item.getReceivedBytes(),
|
||||
item.getTotalBytes(),
|
||||
item.getContentDisposition(),
|
||||
item.getFilename());
|
||||
});
|
||||
if (need_cancel)
|
||||
item.cancel();
|
||||
ipcMain.on('set-download-option', function(event, need_cancel, prevent_default) {
|
||||
window.webContents.session.once('will-download', function(e, item, webContents) {
|
||||
if (prevent_default) {
|
||||
e.preventDefault();
|
||||
const url = item.getURL();
|
||||
const filename = item.getFilename();
|
||||
setImmediate(function() {
|
||||
try {
|
||||
item.getURL();
|
||||
} catch(err) {
|
||||
window.webContents.send('download-error', url, filename, err.message);
|
||||
}
|
||||
});
|
||||
} else {
|
||||
item.setSavePath(downloadFilePath);
|
||||
item.on('done', function(e, state) {
|
||||
window.webContents.send('download-done',
|
||||
state,
|
||||
item.getURL(),
|
||||
item.getMimeType(),
|
||||
item.getReceivedBytes(),
|
||||
item.getTotalBytes(),
|
||||
item.getContentDisposition(),
|
||||
item.getFilename());
|
||||
});
|
||||
if (need_cancel)
|
||||
item.cancel();
|
||||
}
|
||||
});
|
||||
event.returnValue = "done";
|
||||
});
|
||||
});
|
||||
|
|
Loading…
Reference in a new issue