Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: promisify app.getFileIcon() #15742

Merged
merged 12 commits into from Dec 5, 2018
@@ -536,17 +536,15 @@ int ImportIntoCertStore(CertificateManagerModel* model,
#endif

void OnIconDataAvailable(v8::Isolate* isolate,
const App::FileIconCallback& callback,
scoped_refptr<util::Promise> promise,
gfx::Image* icon) {
v8::Locker locker(isolate);
v8::HandleScope handle_scope(isolate);

if (icon && !icon->IsEmpty()) {
callback.Run(v8::Null(isolate), *icon);
promise->Resolve(*icon);
} else {
v8::Local<v8::String> error_message =
v8::String::NewFromUtf8(isolate, "Failed to get file icon.");
callback.Run(v8::Exception::Error(error_message), gfx::Image());
promise->RejectWithErrorMessage("Failed to get file icon.");
}
}

@@ -1125,16 +1123,16 @@ JumpListResult App::SetJumpList(v8::Local<v8::Value> val,
}
#endif // defined(OS_WIN)

void App::GetFileIcon(const base::FilePath& path, mate::Arguments* args) {
mate::Dictionary options;
IconLoader::IconSize icon_size;
FileIconCallback callback;

v8::Local<v8::Promise> App::GetFileIcon(const base::FilePath& path,
mate::Arguments* args) {
v8::Locker locker(isolate());
v8::HandleScope handle_scope(isolate());
v8::HandleScope scope(isolate());
This conversation was marked as resolved by codebytere

This comment has been minimized.

Copy link
@deepak1556

deepak1556 Dec 4, 2018

Member

this line can remain unchanged.


scoped_refptr<util::Promise> promise = new util::Promise(isolate());
base::FilePath normalized_path = path.NormalizePathSeparators();

IconLoader::IconSize icon_size;
mate::Dictionary options;
if (!args->GetNext(&options)) {
icon_size = IconLoader::IconSize::NORMAL;
} else {
@@ -1143,22 +1141,17 @@ void App::GetFileIcon(const base::FilePath& path, mate::Arguments* args) {
icon_size = GetIconSizeByString(icon_size_string);
}

if (!args->GetNext(&callback)) {
args->ThrowError("Missing required callback function");
return;
}

auto* icon_manager = AtomBrowserMainParts::Get()->GetIconManager();
gfx::Image* icon =
icon_manager->LookupIconFromFilepath(normalized_path, icon_size);
if (icon) {
callback.Run(v8::Null(isolate()), *icon);
promise->Resolve(*icon);
} else {
icon_manager->LoadIcon(
normalized_path, icon_size,
base::Bind(&OnIconDataAvailable, isolate(), callback),
&cancelable_task_tracker_);
icon_manager->LoadIcon(normalized_path, icon_size,
base::Bind(&OnIconDataAvailable, isolate(), promise),
&cancelable_task_tracker_);
}
return promise->GetHandle();
}

std::vector<mate::Dictionary> App::GetAppMetrics(v8::Isolate* isolate) {
@@ -198,7 +198,8 @@ class App : public AtomBrowserClient::Delegate,
void ImportCertificate(const base::DictionaryValue& options,
const net::CompletionCallback& callback);
#endif
void GetFileIcon(const base::FilePath& path, mate::Arguments* args);
v8::Local<v8::Promise> GetFileIcon(const base::FilePath& path,
mate::Arguments* args);

std::vector<mate::Dictionary> GetAppMetrics(v8::Isolate* isolate);
v8::Local<v8::Value> GetGPUFeatureStatus(v8::Isolate* isolate);
@@ -552,8 +552,29 @@ On _Windows_, there a 2 kinds of icons:
- Icons associated with certain file extensions, like `.mp3`, `.png`, etc.
- Icons inside the file itself, like `.exe`, `.dll`, `.ico`.

On _Linux_ and _macOS_, icons depend on the application associated with file
mime type.
On _Linux_ and _macOS_, icons depend on the application associated with file mime type.

**[Deprecated Soon](promisification.md)**

### `app.getFileIcon(path[, options])`

* `path` String
* `options` Object (optional)
* `size` String
* `small` - 16x16
* `normal` - 32x32
* `large` - 48x48 on _Linux_, 32x32 on _Windows_, unsupported on _macOS_.

Returns `Promise<NativeImage>` - fulfilled with the app's icon, which is a [NativeImage](native-image.md).

Fetches a path's associated icon.

On _Windows_, there a 2 kinds of icons:

- Icons associated with certain file extensions, like `.mp3`, `.png`, etc.
- Icons inside the file itself, like `.exe`, `.dll`, `.ico`.

On _Linux_ and _macOS_, icons depend on the application associated with file mime type.

### `app.setPath(name, path)`

@@ -40,9 +40,14 @@ Object.assign(app, {
}
})

const nativeFn = app.getAppMetrics
const nativeGetFileIcon = app.getFileIcon
app.getFileIcon = (path, options = {}, cb) => {
return deprecate.promisify(nativeGetFileIcon.call(app, path, options), cb)
}

const nativeAppMetrics = app.getAppMetrics
app.getAppMetrics = () => {
const metrics = nativeFn.call(app)
const metrics = nativeAppMetrics.call(app)
for (const metric of metrics) {
if ('memory' in metric) {
deprecate.removeProperty(metric, 'memory')
@@ -769,61 +769,46 @@ describe('app module', () => {
}
})

it('fetches a non-empty icon', done => {
app.getFileIcon(iconPath, (err, icon) => {
expect(err).to.be.null()
expect(icon.isEmpty()).to.be.false()
done()
})
it('fetches a non-empty icon', async () => {
const icon = await app.getFileIcon(iconPath)
expect(icon.isEmpty()).to.be.false()
})

it('fetches normal icon size by default', done => {
app.getFileIcon(iconPath, (err, icon) => {
const size = icon.getSize()
it('fetches normal icon size by default', async () => {
const icon = await app.getFileIcon(iconPath)
const size = icon.getSize()

expect(err).to.be.null()
expect(size.height).to.equal(sizes.normal)
expect(size.width).to.equal(sizes.normal)
done()
})
expect(size.height).to.equal(sizes.normal)
expect(size.width).to.equal(sizes.normal)
})

describe('size option', () => {
it('fetches a small icon', (done) => {
app.getFileIcon(iconPath, { size: 'small' }, (err, icon) => {
const size = icon.getSize()
expect(err).to.be.null()
expect(size.height).to.equal(sizes.small)
expect(size.width).to.equal(sizes.small)
done()
})
it('fetches a small icon', async () => {
const icon = await app.getFileIcon(iconPath, { size: 'small' })
const size = icon.getSize()

expect(size.height).to.equal(sizes.small)
expect(size.width).to.equal(sizes.small)
})

it('fetches a normal icon', (done) => {
app.getFileIcon(iconPath, { size: 'normal' }, (err, icon) => {
const size = icon.getSize()
expect(err).to.be.null()
expect(size.height).to.equal(sizes.normal)
expect(size.width).to.equal(sizes.normal)
done()
})
it('fetches a normal icon', async () => {
const icon = await app.getFileIcon(iconPath, { size: 'normal' })
console.log(icon)
This conversation was marked as resolved by codebytere

This comment has been minimized.

Copy link
@deepak1556

deepak1556 Dec 4, 2018

Member

remove logging

const size = icon.getSize()

expect(size.height).to.equal(sizes.normal)
expect(size.width).to.equal(sizes.normal)
})

it('fetches a large icon', function (done) {
it('fetches a large icon', async () => {
// macOS does not support large icons
if (process.platform === 'darwin') {
// FIXME(alexeykuzmin): Skip the test.
// this.skip()
return done()
}
if (process.platform === 'darwin') return

app.getFileIcon(iconPath, { size: 'large' }, (err, icon) => {
const size = icon.getSize()
expect(err).to.be.null()
expect(size.height).to.equal(sizes.large)
expect(size.width).to.equal(sizes.large)
done()
})
const icon = await app.getFileIcon(iconPath, { size: 'large' })
const size = icon.getSize()

expect(size.height).to.equal(sizes.large)
expect(size.width).to.equal(sizes.large)
})
})
})
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.