Skip to content

Commit 88f812a

Browse files
yuryuhiroyuki-komatsu
authored andcommitted
Refactor candidate string classes
- Moved class definitions to header files for readability. - Moved each class to separate files and added unit tests. - Added SaveToOutParam and SaveToOptionalOutParam functions to com.h to reduce boilerplate. #codehealth PiperOrigin-RevId: 791964304
1 parent 920842b commit 88f812a

22 files changed

+1068
-516
lines changed

src/base/win32/BUILD.bazel

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,8 +295,9 @@ mozc_cc_library(
295295
tags = MOZC_TAGS.WIN_ONLY,
296296
target_compatible_with = ["@platforms//os:windows"],
297297
deps = [
298+
":hresult",
298299
":hresultor",
299-
"@com_google_absl//absl/meta:type_traits",
300+
"@com_google_absl//absl/base:nullability",
300301
"@com_microsoft_wil//:wil",
301302
],
302303
)

src/base/win32/com.h

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
#include <type_traits>
4343
#include <utility>
4444

45+
#include "absl/base/nullability.h"
4546
#include "base/win32/hresult.h"
4647
#include "base/win32/hresultor.h"
4748

@@ -134,6 +135,71 @@ inline wil::unique_bstr MakeUniqueBSTR(const wchar_t *source) {
134135
return wil::unique_bstr(SysAllocString(source));
135136
}
136137

138+
// Saves the value to the out parameter of a COM method.
139+
//
140+
// Return values:
141+
// - S_OK: success.
142+
// - E_INVALIDARG: the out parameter is null.
143+
// - E_OUTOFMEMORY: the value is a null pointer.
144+
//
145+
// Note: this behavior is different from IUnknown methods. Check COM interface
146+
// documentation to make sure the returned error codes are correct.
147+
template <typename T, typename U>
148+
requires std::integral<T> && std::integral<U>
149+
HResult SaveToOutParam(T value, U *absl_nullable out);
150+
template <typename T, typename U>
151+
HResult SaveToOutParam(T *absl_nullable value, U **absl_nullable out);
152+
template <typename T, typename U>
153+
HResult SaveToOutParam(wil::com_ptr_nothrow<T> value, U **absl_nullable out) {
154+
return SaveToOutParam<T, U>(value.detach(), out);
155+
}
156+
template <typename T>
157+
HResult SaveToOutParam(
158+
wil::unique_any_t<T> value,
159+
typename wil::unique_any_t<T>::pointer *absl_nullable out) {
160+
return SaveToOutParam(value.release(), out);
161+
}
162+
163+
// Saves the value to the out parameter of a COM method. It's a noop if the
164+
// out parameter is nullptr.
165+
template <typename T, typename U>
166+
requires std::integral<T> && std::integral<U>
167+
void SaveToOptionalOutParam(T value, U *absl_nullable out);
168+
169+
// Implementations.
170+
171+
template <typename T, typename U>
172+
requires std::integral<T> && std::integral<U>
173+
HResult SaveToOutParam(T value, U *absl_nullable out) {
174+
static_assert(std::convertible_to<T, U>);
175+
if (out == nullptr) {
176+
return HResultInvalidArg();
177+
}
178+
*out = value;
179+
return HResultOk();
180+
}
181+
182+
template <typename T, typename U>
183+
HResult SaveToOutParam(T *absl_nullable value, U **absl_nullable out) {
184+
if (out == nullptr) {
185+
return HResultInvalidArg();
186+
}
187+
if (value == nullptr) {
188+
return HResultOutOfMemory();
189+
}
190+
*out = value;
191+
return HResultOk();
192+
}
193+
194+
template <typename T, typename U>
195+
requires std::integral<T> && std::integral<U>
196+
void SaveToOptionalOutParam(T value, U *absl_nullable out) {
197+
static_assert(std::convertible_to<T, U>);
198+
if (out != nullptr) {
199+
*out = value;
200+
}
201+
}
202+
137203
} // namespace mozc::win32
138204

139205
#endif // MOZC_BASE_WIN32_COM_H_

src/base/win32/com_test.cc

Lines changed: 60 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@
4848
namespace mozc::win32 {
4949
namespace {
5050

51+
using ::testing::IsNull;
52+
using ::testing::NotNull;
5153
using ::testing::StrEq;
5254

5355
// Mock interfaces for testing.
@@ -113,38 +115,38 @@ class ComTest : public ::testing::Test {
113115
TEST_F(ComTest, ComCreateInstance) {
114116
wil::com_ptr_nothrow<IShellLink> shellink =
115117
ComCreateInstance<IShellLink, ShellLink>();
116-
EXPECT_TRUE(shellink);
117-
EXPECT_TRUE(ComCreateInstance<IShellLink>(CLSID_ShellLink));
118-
EXPECT_FALSE(ComCreateInstance<IShellFolder>(CLSID_ShellLink));
118+
EXPECT_THAT(shellink, NotNull());
119+
EXPECT_THAT(ComCreateInstance<IShellLink>(CLSID_ShellLink), NotNull());
120+
EXPECT_THAT(ComCreateInstance<IShellFolder>(CLSID_ShellLink), IsNull());
119121
}
120122

121123
TEST_F(ComTest, MakeComPtr) {
122124
auto ptr = MakeComPtr<Mock>();
123-
EXPECT_TRUE(ptr);
125+
ASSERT_THAT(ptr, NotNull());
124126
EXPECT_EQ(object_count, 1);
125127
EXPECT_EQ(ptr->GetQICountAndReset(), 0);
126128
}
127129

128130
TEST_F(ComTest, ComQuery) {
129131
wil::com_ptr_nothrow<IMock1> mock1(MakeComPtr<Mock>());
130-
EXPECT_TRUE(mock1);
132+
ASSERT_THAT(mock1, NotNull());
131133
EXPECT_EQ(mock1->Test1(), S_OK);
132134

133135
wil::com_ptr_nothrow<IDerived> derived = ComQuery<IDerived>(mock1);
134-
EXPECT_TRUE(derived);
136+
ASSERT_THAT(derived, NotNull());
135137
EXPECT_EQ(derived->Derived(), 2);
136138
EXPECT_EQ(derived->GetQICountAndReset(), 1);
137139

138-
EXPECT_TRUE(ComQuery<IMock1>(derived));
140+
EXPECT_THAT(ComQuery<IMock1>(derived), NotNull());
139141
EXPECT_EQ(derived->GetQICountAndReset(), 0);
140142

141143
wil::com_ptr_nothrow<IMock2> mock2 = ComQuery<IMock2>(mock1);
142-
EXPECT_TRUE(mock2);
144+
ASSERT_THAT(mock2, NotNull());
143145
EXPECT_EQ(mock2->Test2(), S_FALSE);
144146
EXPECT_EQ(mock1->GetQICountAndReset(), 1);
145147

146148
mock2 = ComQuery<IMock2>(mock1);
147-
EXPECT_TRUE(mock2);
149+
ASSERT_THAT(mock2, NotNull());
148150
EXPECT_EQ(mock2->Test2(), S_FALSE);
149151
EXPECT_EQ(mock1->GetQICountAndReset(), 1);
150152

@@ -154,18 +156,18 @@ TEST_F(ComTest, ComQuery) {
154156

155157
TEST_F(ComTest, ComCopy) {
156158
wil::com_ptr_nothrow<IMock1> mock1(MakeComPtr<Mock>());
157-
EXPECT_TRUE(mock1);
159+
ASSERT_THAT(mock1, NotNull());
158160
EXPECT_EQ(mock1->Test1(), S_OK);
159161

160162
wil::com_ptr_nothrow<IUnknown> unknown = ComCopy<IUnknown>(mock1);
161-
EXPECT_TRUE(unknown);
163+
EXPECT_THAT(unknown, NotNull());
162164
EXPECT_EQ(mock1->GetQICountAndReset(), 0);
163165

164-
EXPECT_FALSE(ComCopy<IShellLink>(unknown));
166+
EXPECT_THAT(ComCopy<IShellLink>(unknown), IsNull());
165167
EXPECT_EQ(mock1->GetQICountAndReset(), 1);
166168

167169
IUnknown *null = nullptr;
168-
EXPECT_FALSE(ComCopy<IUnknown>(null));
170+
EXPECT_THAT(ComCopy<IUnknown>(null), IsNull());
169171
}
170172

171173
TEST(ComBSTRTest, MakeUniqueBSTR) {
@@ -177,5 +179,50 @@ TEST(ComBSTRTest, MakeUniqueBSTR) {
177179
EXPECT_EQ(result.get(), kSource);
178180
}
179181

182+
TEST(SaveToOutParam, Nullptr) {
183+
EXPECT_EQ(SaveToOutParam(0, static_cast<int *>(nullptr)), E_INVALIDARG);
184+
EXPECT_EQ(SaveToOutParam(wil::unique_bstr(), static_cast<BSTR *>(nullptr)),
185+
E_INVALIDARG);
186+
EXPECT_EQ(
187+
SaveToOutParam(static_cast<BSTR>(nullptr), static_cast<BSTR *>(nullptr)),
188+
E_INVALIDARG);
189+
EXPECT_EQ(SaveToOutParam(wil::com_ptr_nothrow<IMock1>(),
190+
static_cast<IMock1 **>(nullptr)),
191+
E_INVALIDARG);
192+
}
193+
194+
TEST(SaveToOutParam, OutOfMemory) {
195+
BSTR out = nullptr;
196+
EXPECT_EQ(SaveToOutParam(static_cast<BSTR>(nullptr), &out), E_OUTOFMEMORY);
197+
}
198+
199+
TEST(SaveToOutParam, BSTR) {
200+
wil::unique_bstr output;
201+
EXPECT_EQ(SaveToOutParam(MakeUniqueBSTR(L"Hello"), output.put()), S_OK);
202+
EXPECT_THAT(output.get(), StrEq(L"Hello"));
203+
}
204+
205+
TEST(SaveToOutParam, ComPtr) {
206+
wil::com_ptr_nothrow<IMock1> output;
207+
EXPECT_EQ(SaveToOutParam(MakeComPtr<Mock>(), output.put()), S_OK);
208+
EXPECT_THAT(output, NotNull());
209+
}
210+
211+
TEST(SaveToOutParam, Int) {
212+
int output = 1;
213+
EXPECT_EQ(SaveToOutParam(0, &output), S_OK);
214+
EXPECT_EQ(output, 0);
215+
}
216+
217+
TEST(SaveToOptionalOutParam, Nullptr) {
218+
SaveToOptionalOutParam(1, static_cast<int *>(nullptr));
219+
}
220+
221+
TEST(SaveToOptionalOutParam, Int) {
222+
int output = 1;
223+
SaveToOptionalOutParam(0, &output);
224+
EXPECT_EQ(output, 0);
225+
}
226+
180227
} // namespace
181228
} // namespace mozc::win32

src/win32/tip/BUILD.bazel

Lines changed: 79 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,8 +219,6 @@ mozc_cc_library(
219219
":tip_thread_context",
220220
":tip_ui_handler",
221221
"//base:const",
222-
"//base:file_util",
223-
"//base:log_file",
224222
"//base:process",
225223
"//base:system_util",
226224
"//base:update_util",
@@ -270,15 +268,74 @@ mozc_cc_library(
270268
],
271269
)
272270

271+
mozc_cc_library(
272+
name = "tip_candidate_string",
273+
srcs = ["tip_candidate_string.cc"],
274+
hdrs = ["tip_candidate_string.h"],
275+
tags = MOZC_TAGS.WIN_ONLY,
276+
target_compatible_with = ["@platforms//os:windows"],
277+
deps = [
278+
":tip_dll_module",
279+
"//base/win32:com",
280+
"@com_google_absl//absl/base:nullability",
281+
],
282+
)
283+
284+
mozc_cc_test(
285+
name = "tip_candidate_string_test",
286+
size = "small",
287+
srcs = ["tip_candidate_string_test.cc"],
288+
tags = MOZC_TAGS.WIN_ONLY,
289+
target_compatible_with = ["@platforms//os:windows"],
290+
deps = [
291+
":gmock_matchers",
292+
":tip_candidate_string",
293+
"//testing:gunit_main",
294+
"@com_microsoft_wil//:wil",
295+
],
296+
)
297+
298+
mozc_cc_library(
299+
name = "tip_enum_candidates",
300+
srcs = ["tip_enum_candidates.cc"],
301+
hdrs = ["tip_enum_candidates.h"],
302+
tags = MOZC_TAGS.WIN_ONLY,
303+
target_compatible_with = ["@platforms//os:windows"],
304+
deps = [
305+
":tip_candidate_string",
306+
":tip_dll_module",
307+
"//base/win32:com",
308+
"@com_google_absl//absl/base:nullability",
309+
],
310+
)
311+
312+
mozc_cc_test(
313+
name = "tip_enum_candidates_test",
314+
size = "small",
315+
srcs = ["tip_enum_candidates_test.cc"],
316+
tags = MOZC_TAGS.WIN_ONLY,
317+
target_compatible_with = ["@platforms//os:windows"],
318+
deps = [
319+
":gmock_matchers",
320+
":tip_enum_candidates",
321+
"//testing:gunit_main",
322+
"@com_microsoft_wil//:wil",
323+
],
324+
)
325+
273326
mozc_cc_library(
274327
name = "tip_candidate_list",
275328
srcs = ["tip_candidate_list.cc"],
276329
hdrs = ["tip_candidate_list.h"],
277330
tags = MOZC_TAGS.WIN_ONLY,
278331
target_compatible_with = ["@platforms//os:windows"],
279332
deps = [
333+
":tip_candidate_string",
280334
":tip_dll_module",
335+
":tip_enum_candidates",
281336
"//base/win32:com",
337+
"@com_google_absl//absl/base:nullability",
338+
"@com_google_absl//absl/functional:any_invocable",
282339
"@com_microsoft_wil//:wil",
283340
],
284341
)
@@ -409,6 +466,7 @@ mozc_cc_library(
409466
":tip_query_provider",
410467
"//base/win32:com",
411468
"//base/win32:com_implements",
469+
"@com_google_absl//absl/base:nullability",
412470
"@com_microsoft_wil//:wil",
413471
],
414472
)
@@ -565,6 +623,7 @@ mozc_cc_library(
565623
":tip_text_service",
566624
"//base/win32:com",
567625
"//base/win32:com_implements",
626+
"@com_google_absl//absl/base:nullability",
568627
"@com_microsoft_wil//:wil",
569628
],
570629
)
@@ -640,6 +699,7 @@ mozc_cc_library(
640699
":tip_text_service",
641700
"//base/win32:com",
642701
"//base/win32:com_implements",
702+
"@com_google_absl//absl/base:nullability",
643703
"@com_microsoft_wil//:wil",
644704
],
645705
)
@@ -755,9 +815,11 @@ mozc_cc_test(
755815
tags = MOZC_TAGS.WIN_ONLY,
756816
target_compatible_with = ["@platforms//os:windows"],
757817
deps = [
818+
":gmock_matchers",
758819
":tip_candidate_list",
759-
":tip_dll_module",
820+
"//base/win32:com",
760821
"//testing:gunit_main",
822+
"@com_google_absl//absl/base:core_headers",
761823
"@com_google_absl//absl/strings",
762824
"@com_microsoft_wil//:wil",
763825
],
@@ -800,3 +862,17 @@ mozc_cc_test(
800862
"//testing:gunit_main",
801863
],
802864
)
865+
866+
mozc_cc_library(
867+
name = "gmock_matchers",
868+
testonly = True,
869+
hdrs = ["gmock_matchers.h"],
870+
tags = MOZC_TAGS.WIN_ONLY,
871+
target_compatible_with = ["@platforms//os:windows"],
872+
deps = [
873+
"//base/win32:hresult",
874+
"//base/win32:wide_char",
875+
"//testing:gunit",
876+
"@com_microsoft_wil//:wil",
877+
],
878+
)

0 commit comments

Comments
 (0)