From 86961d0f4406ba0d6fdca90940da7d083cb187e1 Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Fri, 18 Nov 2016 16:13:43 +0530 Subject: [PATCH 1/5] session: add api to create interrupted downloads --- atom/browser/api/atom_api_download_item.cc | 22 ++++++++- atom/browser/api/atom_api_download_item.h | 5 ++ atom/browser/api/atom_api_session.cc | 53 ++++++++++++++++++++-- atom/browser/api/atom_api_session.h | 1 + 4 files changed, 76 insertions(+), 5 deletions(-) diff --git a/atom/browser/api/atom_api_download_item.cc b/atom/browser/api/atom_api_download_item.cc index 240444d1ee3d..818e18ae58e2 100644 --- a/atom/browser/api/atom_api_download_item.cc +++ b/atom/browser/api/atom_api_download_item.cc @@ -146,6 +146,10 @@ const GURL& DownloadItem::GetURL() const { return download_item_->GetURL(); } +const std::vector& DownloadItem::GetURLChain() const { + return download_item_->GetUrlChain(); +} + content::DownloadItem::DownloadState DownloadItem::GetState() const { return download_item_->GetState(); } @@ -162,6 +166,18 @@ base::FilePath DownloadItem::GetSavePath() const { return save_path_; } +std::string DownloadItem::GetLastModifiedTime() const { + return download_item_->GetLastModifiedTime(); +} + +std::string DownloadItem::GetETag() const { + return download_item_->GetETag(); +} + +double DownloadItem::GetStartTime() const { + return download_item_->GetStartTime().ToDoubleT(); +} + // static void DownloadItem::BuildPrototype(v8::Isolate* isolate, v8::Local prototype) { @@ -180,10 +196,14 @@ void DownloadItem::BuildPrototype(v8::Isolate* isolate, .SetMethod("getFilename", &DownloadItem::GetFilename) .SetMethod("getContentDisposition", &DownloadItem::GetContentDisposition) .SetMethod("getURL", &DownloadItem::GetURL) + .SetMethod("getURLChain", &DownloadItem::GetURLChain) .SetMethod("getState", &DownloadItem::GetState) .SetMethod("isDone", &DownloadItem::IsDone) .SetMethod("setSavePath", &DownloadItem::SetSavePath) - .SetMethod("getSavePath", &DownloadItem::GetSavePath); + .SetMethod("getSavePath", &DownloadItem::GetSavePath) + .SetMethod("getLastModifiedTime", &DownloadItem::GetLastModifiedTime) + .SetMethod("getETag", &DownloadItem::GetETag) + .SetMethod("getStartTime", &DownloadItem::GetStartTime); } // static diff --git a/atom/browser/api/atom_api_download_item.h b/atom/browser/api/atom_api_download_item.h index 332c8cebb877..fbc74b1c818a 100644 --- a/atom/browser/api/atom_api_download_item.h +++ b/atom/browser/api/atom_api_download_item.h @@ -6,6 +6,7 @@ #define ATOM_BROWSER_API_ATOM_API_DOWNLOAD_ITEM_H_ #include +#include #include "atom/browser/api/trackable_object.h" #include "base/files/file_path.h" @@ -38,10 +39,14 @@ class DownloadItem : public mate::TrackableObject, std::string GetFilename() const; std::string GetContentDisposition() const; const GURL& GetURL() const; + const std::vector& GetURLChain() const; content::DownloadItem::DownloadState GetState() const; bool IsDone() const; void SetSavePath(const base::FilePath& path); base::FilePath GetSavePath() const; + std::string GetLastModifiedTime() const; + std::string GetETag() const; + double GetStartTime() const; protected: DownloadItem(v8::Isolate* isolate, content::DownloadItem* download_item); diff --git a/atom/browser/api/atom_api_session.cc b/atom/browser/api/atom_api_session.cc index 96fcca561d9d..ae59619402a1 100644 --- a/atom/browser/api/atom_api_session.cc +++ b/atom/browser/api/atom_api_session.cc @@ -34,6 +34,7 @@ #include "chrome/common/pref_names.h" #include "components/prefs/pref_service.h" #include "content/public/browser/browser_thread.h" +#include "content/public/browser/download_manager_delegate.h" #include "content/public/browser/storage_partition.h" #include "native_mate/dictionary.h" #include "native_mate/object_template_builder.h" @@ -331,6 +332,25 @@ void OnClearStorageDataDone(const base::Closure& callback) { callback.Run(); } +void DownloadIdCallback(content::DownloadManager* download_manager, + const base::FilePath& path, + const std::vector& url_chain, + const std::string& mime_type, + int64_t offset, + int64_t length, + const std::string& last_modified, + const std::string& etag, + const base::Time& start_time, + uint32_t id) { + content::DownloadItem* item = download_manager->CreateDownloadItem( + base::GenerateGUID(), id, path, path, url_chain, GURL(), GURL(), GURL(), + GURL(), mime_type, mime_type, start_time, base::Time(), etag, + last_modified, offset, length, std::string(), + content::DownloadItem::INTERRUPTED, + content::DownloadDangerType::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, + content::DOWNLOAD_INTERRUPT_REASON_NETWORK_TIMEOUT, false); +} + } // namespace Session::Session(v8::Isolate* isolate, AtomBrowserContext* browser_context) @@ -357,10 +377,10 @@ void Session::OnDownloadCreated(content::DownloadManager* manager, v8::Locker locker(isolate()); v8::HandleScope handle_scope(isolate()); - bool prevent_default = Emit( - "will-download", - DownloadItem::Create(isolate(), item), - item->GetWebContents()); + auto handle = DownloadItem::Create(isolate(), item); + if (item->GetState() == content::DownloadItem::INTERRUPTED) + handle->SetSavePath(item->GetTargetFilePath()); + bool prevent_default = Emit("will-download", handle, item->GetWebContents()); if (prevent_default) { item->Cancel(true); item->Remove(); @@ -520,6 +540,29 @@ void Session::GetBlobData( callback)); } +void Session::CreateInterruptedDownload(const mate::Dictionary& options) { + int64_t offset = 0, length = 0; + double start_time = 0.0; + std::string mime_type, last_modified, etag; + base::FilePath path; + std::vector url_chain; + options.Get("path", &path); + options.Get("urlChain", &url_chain); + options.Get("mimeType", &mime_type); + options.Get("offset", &offset); + options.Get("length", &length); + options.Get("lastModified", &last_modified); + options.Get("eTag", &etag); + options.Get("startTime", &start_time); + if (path.empty() || length == 0 || offset >= length) + return; + auto download_manager = + content::BrowserContext::GetDownloadManager(browser_context()); + download_manager->GetDelegate()->GetNextId(base::Bind( + &DownloadIdCallback, download_manager, path, url_chain, mime_type, offset, + length, last_modified, etag, base::Time::FromDoubleT(start_time))); +} + v8::Local Session::Cookies(v8::Isolate* isolate) { if (cookies_.IsEmpty()) { auto handle = Cookies::Create(isolate, browser_context()); @@ -603,6 +646,8 @@ void Session::BuildPrototype(v8::Isolate* isolate, .SetMethod("setUserAgent", &Session::SetUserAgent) .SetMethod("getUserAgent", &Session::GetUserAgent) .SetMethod("getBlobData", &Session::GetBlobData) + .SetMethod("createInterruptedDownload", + &Session::CreateInterruptedDownload) .SetProperty("cookies", &Session::Cookies) .SetProperty("protocol", &Session::Protocol) .SetProperty("webRequest", &Session::WebRequest); diff --git a/atom/browser/api/atom_api_session.h b/atom/browser/api/atom_api_session.h index 8fbdb75fd6ff..6024e135b1cb 100644 --- a/atom/browser/api/atom_api_session.h +++ b/atom/browser/api/atom_api_session.h @@ -79,6 +79,7 @@ class Session: public mate::TrackableObject, std::string GetUserAgent(); void GetBlobData(const std::string& uuid, const AtomBlobReader::CompletionCallback& callback); + void CreateInterruptedDownload(const mate::Dictionary& options); v8::Local Cookies(v8::Isolate* isolate); v8::Local Protocol(v8::Isolate* isolate); v8::Local WebRequest(v8::Isolate* isolate); From d944219b28fb9f0fc9e66cb92bb6109970de26a6 Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Wed, 23 Nov 2016 14:05:26 +0530 Subject: [PATCH 2/5] add docs --- docs/api/download-item.md | 20 ++++++++++++++++++++ docs/api/session.md | 19 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/docs/api/download-item.md b/docs/api/download-item.md index 62df151f9238..25f75ebdb327 100644 --- a/docs/api/download-item.md +++ b/docs/api/download-item.md @@ -146,3 +146,23 @@ header. #### `downloadItem.getState()` Returns `String` - The current state. Can be `progressing`, `completed`, `cancelled` or `interrupted`. + +**Note:** The following methods are useful specifically to resume a +`cancelled` item when session is restarted. + +#### `downloadItem.getURLChain()` + +Returns `String[]` - The complete url chain of the item including any redirects. + +#### `downloadItem.getLastModifiedTime()` + +Returns `String` - Last-Modified header value. + +#### `downloadItem.getETag()` + +Returns `String` - ETag header value. + +#### `downloadItem.getStartTime()` + +Returns `Double` - Number of seconds since the UNIX epoch when the download was +started. diff --git a/docs/api/session.md b/docs/api/session.md index 3f1e41cdaefb..f15f5bbea360 100644 --- a/docs/api/session.md +++ b/docs/api/session.md @@ -344,6 +344,25 @@ Returns `String` - The user agent for this session. Returns `Blob` - The blob data associated with the `identifier`. +#### `ses.createInterruptedDownload(options)` + +* `options` Object + * `path` String - Absolute path of the download. + * `urlChain` String[] - Complete URL chain for the download. + * `mimeType` String (optional) + * `offset` Integer - Start range for the download. + * `length` Integer - Total length of the download. + * `lastModified` String - Last-Modified header value. + * `eTag` String - ETag header value. + * `startTime` Double (optional) - Time when download was started in + number of seconds since UNIX epoch. + +Allows resuming `cancelled` or `interrupted` downloads from previous `Session`. +The API will generate a [DownloadItem](download-item.md) that can be accessed with the [will-download](#event-will-download) +event. The [DownloadItem](download-item.md) will not have any `WebContents` associated with it and +the initial state will be `interrupted`. The download will start only when the +`resume` API is called on the [DownloadItem](download-item.md). + ### Instance Properties The following properties are available on instances of `Session`: From f124732431326031b585a3be0831f9d7556733f5 Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Thu, 24 Nov 2016 20:46:39 +0530 Subject: [PATCH 3/5] add spec --- atom/browser/api/atom_api_session.cc | 4 +- spec/api-session-spec.js | 111 +++++++++++++++++++++++++-- spec/package.json | 1 + spec/static/main.js | 21 ++++- 4 files changed, 125 insertions(+), 12 deletions(-) diff --git a/atom/browser/api/atom_api_session.cc b/atom/browser/api/atom_api_session.cc index ae59619402a1..8ed76e994a16 100644 --- a/atom/browser/api/atom_api_session.cc +++ b/atom/browser/api/atom_api_session.cc @@ -342,7 +342,7 @@ void DownloadIdCallback(content::DownloadManager* download_manager, const std::string& etag, const base::Time& start_time, uint32_t id) { - content::DownloadItem* item = download_manager->CreateDownloadItem( + download_manager->CreateDownloadItem( base::GenerateGUID(), id, path, path, url_chain, GURL(), GURL(), GURL(), GURL(), mime_type, mime_type, start_time, base::Time(), etag, last_modified, offset, length, std::string(), @@ -554,7 +554,7 @@ void Session::CreateInterruptedDownload(const mate::Dictionary& options) { options.Get("lastModified", &last_modified); options.Get("eTag", &etag); options.Get("startTime", &start_time); - if (path.empty() || length == 0 || offset >= length) + if (path.empty() || url_chain.empty() || length == 0 || offset >= length) return; auto download_manager = content::BrowserContext::GetDownloadManager(browser_context()); diff --git a/spec/api-session-spec.js b/spec/api-session-spec.js index 1a75fa333a4a..dfb2db5dedd3 100644 --- a/spec/api-session-spec.js +++ b/spec/api-session-spec.js @@ -3,6 +3,7 @@ const http = require('http') const https = require('https') const path = require('path') const fs = require('fs') +const send = require('send') const {closeWindow} = require('./window-helpers') const {ipcRenderer, remote} = require('electron') @@ -288,7 +289,9 @@ describe('session module', function () { res.end(mockPDF) downloadServer.close() }) - var assertDownload = function (event, state, url, mimeType, receivedBytes, totalBytes, disposition, filename, port, savePath) { + var assertDownload = function (event, state, url, mimeType, + receivedBytes, totalBytes, disposition, + filename, port, savePath) { assert.equal(state, 'completed') assert.equal(filename, 'mock.pdf') assert.equal(savePath, path.join(__dirname, 'fixtures', 'mock.pdf')) @@ -306,8 +309,12 @@ describe('session module', function () { var port = downloadServer.address().port ipcRenderer.sendSync('set-download-option', false, false) w.loadURL(url + ':' + port) - ipcRenderer.once('download-done', function (event, state, url, mimeType, receivedBytes, totalBytes, disposition, filename, savePath) { - assertDownload(event, state, url, mimeType, receivedBytes, totalBytes, disposition, filename, port, savePath) + ipcRenderer.once('download-done', function (event, state, url, + mimeType, receivedBytes, + totalBytes, disposition, + filename, savePath) { + assertDownload(event, state, url, mimeType, receivedBytes, + totalBytes, disposition, filename, port, savePath) done() }) }) @@ -322,8 +329,12 @@ describe('session module', function () { webview.addEventListener('did-finish-load', function () { webview.downloadURL(url + ':' + port + '/') }) - ipcRenderer.once('download-done', function (event, state, url, mimeType, receivedBytes, totalBytes, disposition, filename, savePath) { - assertDownload(event, state, url, mimeType, receivedBytes, totalBytes, disposition, filename, port, savePath) + ipcRenderer.once('download-done', function (event, state, url, + mimeType, receivedBytes, + totalBytes, disposition, + filename, savePath) { + assertDownload(event, state, url, mimeType, receivedBytes, + totalBytes, disposition, filename, port, savePath) document.body.removeChild(webview) done() }) @@ -336,7 +347,10 @@ describe('session module', function () { var port = downloadServer.address().port ipcRenderer.sendSync('set-download-option', true, false) w.loadURL(url + ':' + port + '/') - ipcRenderer.once('download-done', function (event, state, url, mimeType, receivedBytes, totalBytes, disposition, filename) { + ipcRenderer.once('download-done', function (event, state, url, + mimeType, receivedBytes, + totalBytes, disposition, + filename) { assert.equal(state, 'cancelled') assert.equal(filename, 'mock.pdf') assert.equal(mimeType, 'application/pdf') @@ -356,7 +370,10 @@ describe('session module', function () { var port = downloadServer.address().port ipcRenderer.sendSync('set-download-option', true, false) w.loadURL(url + ':' + port + '/?testFilename') - ipcRenderer.once('download-done', function (event, state, url, mimeType, receivedBytes, totalBytes, disposition, filename) { + ipcRenderer.once('download-done', function (event, state, url, + mimeType, receivedBytes, + totalBytes, disposition, + filename) { assert.equal(state, 'cancelled') assert.equal(filename, 'download.pdf') assert.equal(mimeType, 'application/pdf') @@ -565,4 +582,84 @@ describe('session module', function () { w.loadURL(url) }) }) + + describe('ses.createInterruptedDownload(options)', function () { + it('can create an interrupted download item', function (done) { + ipcRenderer.sendSync('set-download-option', true, false) + const filePath = path.join(__dirname, 'fixtures', 'mock.pdf') + const urlChain = ['http://127.0.0.1/'] + const options = { + path: filePath, + urlChain: urlChain, + mimeType: 'application/pdf', + offset: 0, + length: 5242880 + } + w.webContents.session.createInterruptedDownload(options) + ipcRenderer.once('download-created', function (event, state, urlChain, + mimeType, receivedBytes, + totalBytes, filename, + savePath) { + assert.equal(state, 'interrupted') + assert.equal(urlChain, urlChain) + assert.equal(mimeType, 'application/pdf') + assert.equal(receivedBytes, 0) + assert.equal(totalBytes, 5242880) + assert.equal(savePath, filePath) + done() + }) + }) + + it('can be resumed', function (done) { + const fixtures = path.join(__dirname, 'fixtures') + const downloadFilePath = path.join(fixtures, 'logo.png') + const rangeServer = http.createServer(function (req, res) { + let options = { + root: fixtures + } + send(req, req.url, options) + .on('error', function (error) { + done(error) + }).pipe(res) + }) + ipcRenderer.sendSync('set-download-option', true, false, downloadFilePath) + rangeServer.listen(0, '127.0.0.1', function () { + const port = rangeServer.address().port + const downloadUrl = `http://127.0.0.1:${port}/assets/logo.png` + const callback = function (event, state, url, mimeType, + receivedBytes, totalBytes, disposition, + filename, savePath, urlChain, + lastModifiedTime, eTag) { + if (state === 'cancelled') { + const options = { + path: savePath, + urlChain: urlChain, + mimeType: mimeType, + offset: receivedBytes, + length: totalBytes, + lastModified: lastModifiedTime, + eTag: eTag + } + ipcRenderer.sendSync('set-download-option', false, false, downloadFilePath) + w.webContents.session.createInterruptedDownload(options) + } else { + assert.equal(state, 'completed') + assert.equal(filename, 'logo.png') + assert.equal(savePath, downloadFilePath) + assert.equal(url, downloadUrl) + assert.equal(mimeType, 'image/png') + assert.equal(receivedBytes, 14022) + assert.equal(totalBytes, 14022) + assert(fs.existsSync(downloadFilePath)) + fs.unlinkSync(downloadFilePath) + rangeServer.close() + ipcRenderer.removeListener('download-done', callback) + done() + } + } + ipcRenderer.on('download-done', callback) + w.webContents.downloadURL(downloadUrl) + }) + }) + }) }) diff --git a/spec/package.json b/spec/package.json index 8cc1614a27ec..a3b1ba7b26a6 100644 --- a/spec/package.json +++ b/spec/package.json @@ -10,6 +10,7 @@ "mocha": "^3.1.0", "multiparty": "^4.1.2", "q": "^1.4.1", + "send": "^0.14.1", "temp": "^0.8.3", "walkdir": "0.0.11", "ws": "^1.1.1", diff --git a/spec/static/main.js b/spec/static/main.js index ffc13e39a8ee..f29cbebeeb1e 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -137,8 +137,16 @@ 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, needCancel, preventDefault) { + ipcMain.on('set-download-option', function (event, needCancel, preventDefault, filePath = downloadFilePath) { window.webContents.session.once('will-download', function (e, item) { + window.webContents.send('download-created', + item.getState(), + item.getURLChain(), + item.getMimeType(), + item.getReceivedBytes(), + item.getTotalBytes(), + item.getFilename(), + item.getSavePath()) if (preventDefault) { e.preventDefault() const url = item.getURL() @@ -151,7 +159,11 @@ app.on('ready', function () { } }) } else { - item.setSavePath(downloadFilePath) + if (item.getState() === 'interrupted' && !needCancel) { + item.resume() + } else { + item.setSavePath(filePath) + } item.on('done', function (e, state) { window.webContents.send('download-done', state, @@ -161,7 +173,10 @@ app.on('ready', function () { item.getTotalBytes(), item.getContentDisposition(), item.getFilename(), - item.getSavePath()) + item.getSavePath(), + item.getURLChain(), + item.getLastModifiedTime(), + item.getETag()) }) if (needCancel) item.cancel() } From 5d94221c61b196a16001ce0cff192e1274556919 Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Wed, 7 Dec 2016 12:58:53 +0530 Subject: [PATCH 4/5] fix code style --- atom/browser/api/atom_api_session.cc | 10 +++++++++- spec/api-session-spec.js | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/atom/browser/api/atom_api_session.cc b/atom/browser/api/atom_api_session.cc index 8ed76e994a16..8cb9d3395e5c 100644 --- a/atom/browser/api/atom_api_session.cc +++ b/atom/browser/api/atom_api_session.cc @@ -554,8 +554,16 @@ void Session::CreateInterruptedDownload(const mate::Dictionary& options) { options.Get("lastModified", &last_modified); options.Get("eTag", &etag); options.Get("startTime", &start_time); - if (path.empty() || url_chain.empty() || length == 0 || offset >= length) + if (path.empty() || url_chain.empty() || length == 0) { + isolate()->ThrowException(v8::Exception::Error(mate::StringToV8( + isolate(), "Must pass non-empty path, urlChain and length."))); return; + } + if (offset >= length) { + isolate()->ThrowException(v8::Exception::Error(mate::StringToV8( + isolate(), "Must pass an offset value less than length."))); + return; + } auto download_manager = content::BrowserContext::GetDownloadManager(browser_context()); download_manager->GetDelegate()->GetNextId(base::Bind( diff --git a/spec/api-session-spec.js b/spec/api-session-spec.js index dfb2db5dedd3..b785e8cb0ff6 100644 --- a/spec/api-session-spec.js +++ b/spec/api-session-spec.js @@ -601,7 +601,7 @@ describe('session module', function () { totalBytes, filename, savePath) { assert.equal(state, 'interrupted') - assert.equal(urlChain, urlChain) + assert.deepEqual(urlChain, urlChain) assert.equal(mimeType, 'application/pdf') assert.equal(receivedBytes, 0) assert.equal(totalBytes, 5242880) From d705f4cbac4f4074a15fe6f8977e1696d8bcaa24 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Fri, 9 Dec 2016 09:36:44 -0800 Subject: [PATCH 5/5] Fix issue where actual/expected was same variable --- spec/api-session-spec.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/spec/api-session-spec.js b/spec/api-session-spec.js index b785e8cb0ff6..f7a5330fa7b3 100644 --- a/spec/api-session-spec.js +++ b/spec/api-session-spec.js @@ -587,10 +587,9 @@ describe('session module', function () { it('can create an interrupted download item', function (done) { ipcRenderer.sendSync('set-download-option', true, false) const filePath = path.join(__dirname, 'fixtures', 'mock.pdf') - const urlChain = ['http://127.0.0.1/'] const options = { path: filePath, - urlChain: urlChain, + urlChain: ['http://127.0.0.1/'], mimeType: 'application/pdf', offset: 0, length: 5242880 @@ -601,7 +600,7 @@ describe('session module', function () { totalBytes, filename, savePath) { assert.equal(state, 'interrupted') - assert.deepEqual(urlChain, urlChain) + assert.deepEqual(urlChain, ['http://127.0.0.1/']) assert.equal(mimeType, 'application/pdf') assert.equal(receivedBytes, 0) assert.equal(totalBytes, 5242880)