1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
|
From 47640eaa9a5cd3d4ed53f841f34f30c49f89cff9 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles@charleskerr.com>
Date: Mon, 25 Jan 2021 14:44:04 -0600
Subject: [PATCH] fix: don't throw on bad icons in BrowserWindow constructor
(#27441) (#27478)
---
shell/browser/api/electron_api_base_window.cc | 11 ++++++--
shell/browser/api/electron_api_base_window.h | 3 ++
shell/common/api/electron_api_native_image.cc | 28 ++++++++++++++-----
shell/common/api/electron_api_native_image.h | 10 +++++--
spec-main/api-browser-window-spec.ts | 9 ++++++
5 files changed, 49 insertions(+), 12 deletions(-)
diff --git a/shell/browser/api/electron_api_base_window.cc b/shell/browser/api/electron_api_base_window.cc
index 4995db43e..d053ed6e8 100644
--- a/shell/browser/api/electron_api_base_window.cc
+++ b/shell/browser/api/electron_api_base_window.cc
@@ -101,7 +101,7 @@ BaseWindow::BaseWindow(v8::Isolate* isolate,
#if defined(TOOLKIT_VIEWS)
v8::Local<v8::Value> icon;
if (options.Get(options::kIcon, &icon)) {
- SetIcon(isolate, icon);
+ SetIconImpl(isolate, icon, NativeImage::OnConvertError::kWarn);
}
#endif
}
@@ -999,8 +999,15 @@ bool BaseWindow::SetThumbarButtons(gin_helper::Arguments* args) {
#if defined(TOOLKIT_VIEWS)
void BaseWindow::SetIcon(v8::Isolate* isolate, v8::Local<v8::Value> icon) {
+ SetIconImpl(isolate, icon, NativeImage::OnConvertError::kThrow);
+}
+
+void BaseWindow::SetIconImpl(v8::Isolate* isolate,
+ v8::Local<v8::Value> icon,
+ NativeImage::OnConvertError on_error) {
NativeImage* native_image = nullptr;
- if (!NativeImage::TryConvertNativeImage(isolate, icon, &native_image))
+ if (!NativeImage::TryConvertNativeImage(isolate, icon, &native_image,
+ on_error))
return;
#if defined(OS_WIN)
diff --git a/shell/browser/api/electron_api_base_window.h b/shell/browser/api/electron_api_base_window.h
index 912c2306f..2276e83b5 100644
--- a/shell/browser/api/electron_api_base_window.h
+++ b/shell/browser/api/electron_api_base_window.h
@@ -222,6 +222,9 @@ class BaseWindow : public gin_helper::TrackableObject<BaseWindow>,
bool SetThumbarButtons(gin_helper::Arguments* args);
#if defined(TOOLKIT_VIEWS)
void SetIcon(v8::Isolate* isolate, v8::Local<v8::Value> icon);
+ void SetIconImpl(v8::Isolate* isolate,
+ v8::Local<v8::Value> icon,
+ NativeImage::OnConvertError on_error);
#endif
#if defined(OS_WIN)
typedef base::RepeatingCallback<void(v8::Local<v8::Value>,
diff --git a/shell/common/api/electron_api_native_image.cc b/shell/common/api/electron_api_native_image.cc
index c786d0eb8..2c087a313 100644
--- a/shell/common/api/electron_api_native_image.cc
+++ b/shell/common/api/electron_api_native_image.cc
@@ -10,6 +10,7 @@
#include <vector>
#include "base/files/file_util.h"
+#include "base/logging.h"
#include "base/strings/pattern.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
@@ -140,7 +141,10 @@ NativeImage::~NativeImage() {
// static
bool NativeImage::TryConvertNativeImage(v8::Isolate* isolate,
v8::Local<v8::Value> image,
- NativeImage** native_image) {
+ NativeImage** native_image,
+ OnConvertError on_error) {
+ std::string error_message;
+
base::FilePath icon_path;
if (gin::ConvertFromV8(isolate, image, &icon_path)) {
*native_image = NativeImage::CreateFromPath(isolate, icon_path).get();
@@ -150,17 +154,27 @@ bool NativeImage::TryConvertNativeImage(v8::Isolate* isolate,
#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;
+ error_message = "Failed to load image from path '" + img_path + "'";
}
} 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;
+ error_message = "Argument must be a file path or a NativeImage";
}
}
+
+ if (!error_message.empty()) {
+ switch (on_error) {
+ case OnConvertError::kThrow:
+ isolate->ThrowException(
+ v8::Exception::Error(gin::StringToV8(isolate, error_message)));
+ break;
+ case OnConvertError::kWarn:
+ LOG(WARNING) << error_message;
+ break;
+ }
+ return false;
+ }
+
return true;
}
diff --git a/shell/common/api/electron_api_native_image.h b/shell/common/api/electron_api_native_image.h
index 1c89fcdfa..f0ecb35cc 100644
--- a/shell/common/api/electron_api_native_image.h
+++ b/shell/common/api/electron_api_native_image.h
@@ -77,9 +77,13 @@ 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);
+ enum class OnConvertError { kThrow, kWarn };
+
+ static bool TryConvertNativeImage(
+ v8::Isolate* isolate,
+ v8::Local<v8::Value> image,
+ NativeImage** native_image,
+ OnConvertError on_error = OnConvertError::kThrow);
// gin::Wrappable
static gin::WrapperInfo kWrapperInfo;
diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts
index 379cdff04..7e490427c 100644
--- a/spec-main/api-browser-window-spec.ts
+++ b/spec-main/api-browser-window-spec.ts
@@ -62,6 +62,15 @@ describe('BrowserWindow module', () => {
const appProcess = childProcess.spawn(process.execPath, [appPath]);
await new Promise((resolve) => { appProcess.once('exit', resolve); });
});
+
+ it('does not crash or throw when passed an invalid icon', async () => {
+ expect(() => {
+ const w = new BrowserWindow({
+ icon: undefined
+ } as any);
+ w.destroy();
+ }).not.to.throw();
+ });
});
describe('garbage collection', () => {
--
2.30.0
|