mirrors/electron
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor: remove path from nativeImage converter (#26546)

Browse source
This commit is contained in:
Shelley Vohr 2021-01-04 12:58:31 -08:00 committed by GitHub
parent 4db3e3a08a
commit 3455136e9d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 167 additions and 103 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

22
shell/browser/api/electron_api_base_window.cc
View file

@ -99,13 +99,9 @@ BaseWindow::BaseWindow(v8::Isolate* isolate,
window_->AddObserver(this);
#if defined(TOOLKIT_VIEWS)
{
v8::TryCatch try_catch(isolate);
gin::Handle<NativeImage> icon;
if (options.Get(options::kIcon, &icon) && !icon.IsEmpty())
SetIcon(icon);
if (try_catch.HasCaught())
LOG(ERROR) << "Failed to convert NativeImage";
v8::Local<v8::Value> icon;
if (options.Get(options::kIcon, &icon)) {
SetIcon(isolate, icon);
}
#endif
}
@ -1003,14 +999,18 @@ bool BaseWindow::SetThumbarButtons(gin_helper::Arguments* args) {
}
#if defined(TOOLKIT_VIEWS)
void BaseWindow::SetIcon(gin::Handle<NativeImage> icon) {
void BaseWindow::SetIcon(v8::Isolate* isolate, v8::Local<v8::Value> icon) {
NativeImage* native_image = nullptr;
if (!NativeImage::TryConvertNativeImage(isolate, icon, &native_image))
return;
#if defined(OS_WIN)
static_cast<NativeWindowViews*>(window_.get())
->SetIcon(icon->GetHICON(GetSystemMetrics(SM_CXSMICON)),
icon->GetHICON(GetSystemMetrics(SM_CXICON)));
->SetIcon(native_image->GetHICON(GetSystemMetrics(SM_CXSMICON)),
native_image->GetHICON(GetSystemMetrics(SM_CXICON)));
#elif defined(OS_LINUX)
static_cast<NativeWindowViews*>(window_.get())
->SetIcon(icon->image().AsImageSkia());
->SetIcon(native_image->image().AsImageSkia());
#endif
}
#endif

3
shell/browser/api/electron_api_base_window.h
View file

@ -17,6 +17,7 @@
#include "shell/browser/native_window.h"
#include "shell/browser/native_window_observer.h"
#include "shell/common/api/electron_api_native_image.h"
#include "shell/common/gin_helper/error_thrower.h"
#include "shell/common/gin_helper/trackable_object.h"
namespace electron {
@ -221,7 +222,7 @@ class BaseWindow : public gin_helper::TrackableObject<BaseWindow>,
// Extra APIs added in JS.
bool SetThumbarButtons(gin_helper::Arguments* args);
#if defined(TOOLKIT_VIEWS)
void SetIcon(gin::Handle<NativeImage> icon);
void SetIcon(v8::Isolate* isolate, v8::Local<v8::Value> icon);
#endif
#if defined(OS_WIN)
typedef base::RepeatingCallback<void(v8::Local<v8::Value>,

49
shell/browser/api/electron_api_tray.cc
View file

@ -14,6 +14,7 @@
#include "shell/browser/browser.h"
#include "shell/browser/javascript_environment.h"
#include "shell/common/api/electron_api_native_image.h"
#include "shell/common/gin_converters/file_path_converter.h"
#include "shell/common/gin_converters/gfx_converter.h"
#include "shell/common/gin_converters/guid_converter.h"
#include "shell/common/gin_converters/image_converter.h"
@ -61,11 +62,11 @@ namespace api {
gin::WrapperInfo Tray::kWrapperInfo = {gin::kEmbedderNativeGin};
Tray::Tray(gin::Handle<NativeImage> image,
base::Optional<UUID> guid,
gin::Arguments* args)
Tray::Tray(v8::Isolate* isolate,
v8::Local<v8::Value> image,
base::Optional<UUID> guid)
: tray_icon_(TrayIcon::Create(guid)) {
SetImage(image);
SetImage(isolate, image);
tray_icon_->AddObserver(this);
}
@ -73,7 +74,7 @@ Tray::~Tray() = default;
// static
gin::Handle<Tray> Tray::New(gin_helper::ErrorThrower thrower,
gin::Handle<NativeImage> image,
v8::Local<v8::Value> image,
base::Optional<UUID> guid,
gin::Arguments* args) {
if (!Browser::Get()->is_ready()) {
@ -88,7 +89,8 @@ gin::Handle<Tray> Tray::New(gin_helper::ErrorThrower thrower,
}
#endif
return gin::CreateHandle(thrower.isolate(), new Tray(image, guid, args));
return gin::CreateHandle(thrower.isolate(),
new Tray(args->isolate(), image, guid));
}
void Tray::OnClicked(const gfx::Rect& bounds,
@ -186,23 +188,34 @@ bool Tray::IsDestroyed() {
return !tray_icon_;
}
void Tray::SetImage(gin::Handle<NativeImage> image) {
void Tray::SetImage(v8::Isolate* isolate, v8::Local<v8::Value> image) {
if (!CheckAlive())
return;
NativeImage* native_image = nullptr;
if (!NativeImage::TryConvertNativeImage(isolate, image, &native_image))
return;
#if defined(OS_WIN)
tray_icon_->SetImage(image->GetHICON(GetSystemMetrics(SM_CXSMICON)));
tray_icon_->SetImage(native_image->GetHICON(GetSystemMetrics(SM_CXSMICON)));
#else
tray_icon_->SetImage(image->image());
tray_icon_->SetImage(native_image->image());
#endif
}
void Tray::SetPressedImage(gin::Handle<NativeImage> image) {
void Tray::SetPressedImage(v8::Isolate* isolate, v8::Local<v8::Value> image) {
if (!CheckAlive())
return;
NativeImage* native_image = nullptr;
if (!NativeImage::TryConvertNativeImage(isolate, image, &native_image))
return;
#if defined(OS_WIN)
tray_icon_->SetPressedImage(image->GetHICON(GetSystemMetrics(SM_CXSMICON)));
tray_icon_->SetPressedImage(
native_image->GetHICON(GetSystemMetrics(SM_CXSMICON)));
#else
tray_icon_->SetPressedImage(image->image());
tray_icon_->SetPressedImage(native_image->image());
#endif
}
@ -282,14 +295,20 @@ void Tray::DisplayBalloon(gin_helper::ErrorThrower thrower,
return;
}
gin::Handle<NativeImage> icon;
options.Get("icon", &icon);
v8::Local<v8::Value> icon_value;
NativeImage* icon = nullptr;
if (options.Get("icon", &icon_value) &&
!NativeImage::TryConvertNativeImage(thrower.isolate(), icon_value,
&icon)) {
return;
}
options.Get("iconType", &balloon_options.icon_type);
options.Get("largeIcon", &balloon_options.large_icon);
options.Get("noSound", &balloon_options.no_sound);
options.Get("respectQuietTime", &balloon_options.respect_quiet_time);
if (!icon.IsEmpty()) {
if (icon) {
#if defined(OS_WIN)
balloon_options.icon = icon->GetHICON(
GetSystemMetrics(balloon_options.large_icon ? SM_CXICON : SM_CXSMICON));

12
shell/browser/api/electron_api_tray.h
View file

@ -43,7 +43,7 @@ class Tray : public gin::Wrappable<Tray>,
public:
// gin_helper::Constructible
static gin::Handle<Tray> New(gin_helper::ErrorThrower thrower,
gin::Handle<NativeImage> image,
v8::Local<v8::Value> image,
base::Optional<UUID> guid,
gin::Arguments* args);
static v8::Local<v8::ObjectTemplate> FillObjectTemplate(
@ -54,9 +54,9 @@ class Tray : public gin::Wrappable<Tray>,
static gin::WrapperInfo kWrapperInfo;
private:
Tray(gin::Handle<NativeImage> image,
base::Optional<UUID> guid,
gin::Arguments* args);
Tray(v8::Isolate* isolate,
v8::Local<v8::Value> image,
base::Optional<UUID> guid);
~Tray() override;
// TrayIconObserver:
@ -83,8 +83,8 @@ class Tray : public gin::Wrappable<Tray>,
// JS API:
void Destroy();
bool IsDestroyed();
void SetImage(gin::Handle<NativeImage> image);
void SetPressedImage(gin::Handle<NativeImage> image);
void SetImage(v8::Isolate* isolate, v8::Local<v8::Value> image);
void SetPressedImage(v8::Isolate* isolate, v8::Local<v8::Value> image);
void SetToolTip(const std::string& tool_tip);
void SetTitle(const std::string& title,
const base::Optional<gin_helper::Dictionary>& options,

12
shell/browser/api/electron_api_web_contents.cc
View file

@ -2819,10 +2819,16 @@ void WebContents::StartDrag(const gin_helper::Dictionary& item,
files.push_back(file);
}
gin::Handle<NativeImage> icon;
if (!item.Get("icon", &icon) || icon->image().IsEmpty()) {
v8::Local<v8::Value> icon_value;
if (!item.Get("icon", &icon_value)) {
gin_helper::ErrorThrower(args->isolate())
.ThrowError("Must specify non-empty 'icon' option");
.ThrowError("'icon' parameter is required");
return;
}
NativeImage* icon = nullptr;
if (!NativeImage::TryConvertNativeImage(args->isolate(), icon_value, &icon) ||
icon->image().IsEmpty()) {
return;
}

2
shell/browser/browser.h
View file

@ -223,7 +223,7 @@ class Browser : public WindowListObserver {
void DockSetMenu(ElectronMenuModel* model);
// Set docks' icon.
void DockSetIcon(const gfx::Image& image);
void DockSetIcon(v8::Isolate* isolate, v8::Local<v8::Value> icon);
#endif // defined(OS_MAC)

12
shell/browser/browser_mac.mm
View file

@ -20,6 +20,7 @@
#include "shell/browser/mac/electron_application_delegate.h"
#include "shell/browser/native_window.h"
#include "shell/browser/window_list.h"
#include "shell/common/api/electron_api_native_image.h"
#include "shell/common/application_info.h"
#include "shell/common/gin_converters/image_converter.h"
#include "shell/common/gin_helper/arguments.h"
@ -454,7 +455,16 @@ void Browser::DockSetMenu(ElectronMenuModel* model) {
[delegate setApplicationDockMenu:model];
}
void Browser::DockSetIcon(const gfx::Image& image) {
void Browser::DockSetIcon(v8::Isolate* isolate, v8::Local<v8::Value> icon) {
gfx::Image image;
if (!icon->IsNull()) {
api::NativeImage* native_image = nullptr;
if (!api::NativeImage::TryConvertNativeImage(isolate, icon, &native_image))
return;
image = native_image->image();
}
[[AtomApplication sharedApplication]
setApplicationIconImage:image.AsNSImage()];
}

72
shell/common/api/electron_api_native_image.cc
View file

@ -17,6 +17,7 @@
#include "gin/arguments.h"
#include "gin/object_template_builder.h"
#include "gin/per_isolate_data.h"
#include "gin/wrappable.h"
#include "net/base/data_url.h"
#include "shell/common/asar/asar_util.h"
#include "shell/common/gin_converters/file_path_converter.h"
@ -134,6 +135,33 @@ NativeImage::~NativeImage() {
}
}
// static
bool NativeImage::TryConvertNativeImage(v8::Isolate* isolate,
v8::Local<v8::Value> image,
NativeImage** native_image) {
base::FilePath icon_path;
if (gin::ConvertFromV8(isolate, image, &icon_path)) {
*native_image = NativeImage::CreateFromPath(isolate, icon_path).get();
if ((*native_image)->image().IsEmpty()) {
#if defined(OS_WIN)
const auto img_path = base::UTF16ToUTF8(icon_path.value());
#else
const auto img_path = icon_path.value();
#endif
isolate->ThrowException(v8::Exception::Error(gin::StringToV8(
isolate, "Failed to load image from path '" + img_path + "'")));
return false;
}
} else {
if (!gin::ConvertFromV8(isolate, image, native_image)) {
isolate->ThrowException(v8::Exception::Error(gin::StringToV8(
isolate, "Argument must be a file path or a NativeImage")));
return false;
}
}
return true;
}
#if defined(OS_WIN)
HICON NativeImage::GetHICON(int size) {
auto iter = hicons_.find(size);
@ -571,50 +599,6 @@ gin::WrapperInfo NativeImage::kWrapperInfo = {gin::kEmbedderNativeGin};
} // namespace electron
namespace gin {
v8::Local<v8::Value> Converter<electron::api::NativeImage*>::ToV8(
v8::Isolate* isolate,
electron::api::NativeImage* val) {
v8::Local<v8::Object> ret;
if (val && val->GetWrapper(isolate).ToLocal(&ret))
return ret;
else
return v8::Null(isolate);
}
bool Converter<electron::api::NativeImage*>::FromV8(
v8::Isolate* isolate,
v8::Local<v8::Value> val,
electron::api::NativeImage** out) {
// Try converting from file path.
base::FilePath path;
if (ConvertFromV8(isolate, val, &path)) {
*out = electron::api::NativeImage::CreateFromPath(isolate, path).get();
if ((*out)->image().IsEmpty()) {
#if defined(OS_WIN)
const auto img_path = base::UTF16ToUTF8(path.value());
#else
const auto img_path = path.value();
#endif
isolate->ThrowException(v8::Exception::Error(
StringToV8(isolate, "Image could not be created from " + img_path)));
return false;
}
return true;
}
// reinterpret_cast is safe here because NativeImage is the only subclass of
// gin::Wrappable<NativeImage>.
*out = static_cast<electron::api::NativeImage*>(
static_cast<gin::WrappableBase*>(gin::internal::FromV8Impl(
isolate, val, &electron::api::NativeImage::kWrapperInfo)));
return *out != nullptr;
}
} // namespace gin
namespace {
using electron::api::NativeImage;

18
shell/common/api/electron_api_native_image.h
View file

@ -77,6 +77,10 @@ class NativeImage : public gin::Wrappable<NativeImage> {
static v8::Local<v8::FunctionTemplate> GetConstructor(v8::Isolate* isolate);
static bool TryConvertNativeImage(v8::Isolate* isolate,
v8::Local<v8::Value> image,
NativeImage** native_image);
// gin::Wrappable
static gin::WrapperInfo kWrapperInfo;
gin::ObjectTemplateBuilder GetObjectTemplateBuilder(
@ -133,18 +137,4 @@ class NativeImage : public gin::Wrappable<NativeImage> {
} // namespace electron
namespace gin {
// A custom converter that allows converting path to NativeImage.
template <>
struct Converter<electron::api::NativeImage*> {
static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
electron::api::NativeImage* val);
static bool FromV8(v8::Isolate* isolate,
v8::Local<v8::Value> val,
electron::api::NativeImage** out);
};
} // namespace gin
#endif // SHELL_COMMON_API_ELECTRON_API_NATIVE_IMAGE_H_

12
shell/common/gin_converters/image_converter.cc
View file

@ -27,9 +27,19 @@ bool Converter<gfx::Image>::FromV8(v8::Isolate* isolate,
if (val->IsNull())
return true;
gin::Handle<electron::api::NativeImage> native_image;
// First see if the user has passed a path.
electron::api::NativeImage* native_image = nullptr;
base::FilePath icon_path;
if (gin::ConvertFromV8(isolate, val, &icon_path)) {
native_image =
electron::api::NativeImage::CreateFromPath(isolate, icon_path).get();
if (native_image->image().IsEmpty())
return false;
} else {
// Try a normal nativeImage if that fails.
if (!gin::ConvertFromV8(isolate, val, &native_image))
return false;
}
*out = native_image->image();
return true;

9
spec-main/api-app-spec.ts
View file

@ -1541,6 +1541,15 @@ describe('app module', () => {
});
});
describe('dock.setIcon', () => {
it('throws a descriptive error for a bad icon path', () => {
const badPath = path.resolve('I', 'Do', 'Not', 'Exist');
expect(() => {
app.dock.setIcon(badPath);
}).to.throw(/Failed to load image from path (.+)/);
});
});
describe('dock.bounce', () => {
it('should return -1 for unknown bounce type', () => {
expect(app.dock.bounce('bad type' as any)).to.equal(-1);

39
spec-main/api-tray-spec.ts
View file

@ -19,10 +19,10 @@ describe('tray module', () => {
const badPath = path.resolve('I', 'Do', 'Not', 'Exist');
expect(() => {
tray = new Tray(badPath);
}).to.throw(/Error processing argument at index 0/);
}).to.throw(/Failed to load image from path (.+)/);
});
ifit(process.platform === 'win32')('throws a descriptive error if an invlaid guid is given', () => {
ifit(process.platform === 'win32')('throws a descriptive error if an invalid guid is given', () => {
expect(() => {
tray = new Tray(nativeImage.createEmpty(), 'I am not a guid');
}).to.throw('Invalid GUID format');
@ -132,17 +132,52 @@ describe('tray module', () => {
});
describe('tray.setImage(image)', () => {
it('throws a descriptive error for a missing file', () => {
const badPath = path.resolve('I', 'Do', 'Not', 'Exist');
expect(() => {
tray.setImage(badPath);
}).to.throw(/Failed to load image from path (.+)/);
});
it('accepts empty image', () => {
tray.setImage(nativeImage.createEmpty());
});
});
describe('tray.setPressedImage(image)', () => {
it('throws a descriptive error for a missing file', () => {
const badPath = path.resolve('I', 'Do', 'Not', 'Exist');
expect(() => {
tray.setPressedImage(badPath);
}).to.throw(/Failed to load image from path (.+)/);
});
it('accepts empty image', () => {
tray.setPressedImage(nativeImage.createEmpty());
});
});
ifdescribe(process.platform === 'win32')('tray.displayBalloon(image)', () => {
it('throws a descriptive error for a missing file', () => {
const badPath = path.resolve('I', 'Do', 'Not', 'Exist');
expect(() => {
tray.displayBalloon({
title: 'title',
content: 'wow content',
icon: badPath
});
}).to.throw(/Failed to load image from path (.+)/);
});
it('accepts an empty image', () => {
tray.displayBalloon({
title: 'title',
content: 'wow content',
icon: nativeImage.createEmpty()
});
});
});
ifdescribe(process.platform === 'darwin')('tray get/set title', () => {
it('sets/gets non-empty title', () => {
const title = 'Hello World!';

4
spec-main/api-web-contents-spec.ts
View file

@ -764,11 +764,11 @@ describe('webContents module', () => {
expect(() => {
w.webContents.startDrag({ file: __filename } as any);
}).to.throw('Must specify non-empty \'icon\' option');
}).to.throw('\'icon\' parameter is required');
expect(() => {
w.webContents.startDrag({ file: __filename, icon: path.join(mainFixturesPath, 'blank.png') });
}).to.throw('Must specify non-empty \'icon\' option');
}).to.throw(/Failed to load image from path (.+)/);
});
});