From a620b611d0857d6b5f3b0ed923b4fe2ee5a9a019 Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Thu, 12 Jan 2023 16:28:59 -0800 Subject: [PATCH] [windows] Use FML_DCHECK in place of C assert Now that the embedders depend on FML, migrate assertions to FML_DCHECK, which allows for an explanatory log message to be emitted along with the assertion. No tests since no semantic changes are made. --- .../windows/accessibility_bridge_windows.cc | 8 +++++--- .../flutter_platform_node_delegate_windows.cc | 18 ++++++++++------- shell/platform/windows/flutter_windows.cc | 1 - .../windows/keyboard_key_embedder_handler.cc | 20 +++++++++---------- .../platform/windows/keyboard_key_handler.cc | 5 +++-- shell/platform/windows/keyboard_manager.cc | 8 ++++---- shell/platform/windows/keyboard_unittests.cc | 10 ++++++---- shell/platform/windows/keyboard_utils.cc | 6 ++++-- shell/platform/windows/keyboard_utils.h | 2 +- shell/platform/windows/platform_handler.cc | 4 ++-- .../windows/testing/test_binary_messenger.h | 4 ++-- .../platform/windows/testing/test_keyboard.cc | 9 ++++++--- shell/platform/windows/text_input_manager.cc | 11 ++++------ 13 files changed, 58 insertions(+), 48 deletions(-) diff --git a/shell/platform/windows/accessibility_bridge_windows.cc b/shell/platform/windows/accessibility_bridge_windows.cc index d5b252ae8d43a..fbdb79b9d1c94 100644 --- a/shell/platform/windows/accessibility_bridge_windows.cc +++ b/shell/platform/windows/accessibility_bridge_windows.cc @@ -4,6 +4,7 @@ #include "flutter/shell/platform/windows/accessibility_bridge_windows.h" +#include "flutter/fml/logging.h" #include "flutter/shell/platform/windows/flutter_platform_node_delegate_windows.h" #include "flutter/third_party/accessibility/ax/platform/ax_platform_node_delegate_base.h" @@ -13,8 +14,8 @@ AccessibilityBridgeWindows::AccessibilityBridgeWindows( FlutterWindowsEngine* engine, FlutterWindowsView* view) : engine_(engine), view_(view) { - assert(engine_); - assert(view_); + FML_DCHECK(engine_); + FML_DCHECK(view_); } void AccessibilityBridgeWindows::OnAccessibilityEvent( @@ -24,7 +25,8 @@ void AccessibilityBridgeWindows::OnAccessibilityEvent( auto node_delegate = GetFlutterPlatformNodeDelegateFromID(ax_node->id()).lock(); - assert(node_delegate); + FML_DCHECK(node_delegate) + << "No FlutterPlatformNodeDelegate found for node ID " << ax_node->id(); std::shared_ptr win_delegate = std::static_pointer_cast( node_delegate); diff --git a/shell/platform/windows/flutter_platform_node_delegate_windows.cc b/shell/platform/windows/flutter_platform_node_delegate_windows.cc index 0e2ce94c7a7e2..57b9eb91682d0 100644 --- a/shell/platform/windows/flutter_platform_node_delegate_windows.cc +++ b/shell/platform/windows/flutter_platform_node_delegate_windows.cc @@ -6,6 +6,7 @@ #include "flutter/shell/platform/windows/flutter_platform_node_delegate_windows.h" +#include "flutter/fml/logging.h" #include "flutter/shell/platform/windows/accessibility_bridge_windows.h" #include "flutter/shell/platform/windows/flutter_windows_view.h" #include "flutter/third_party/accessibility/ax/ax_clipping_behavior.h" @@ -18,8 +19,9 @@ FlutterPlatformNodeDelegateWindows::FlutterPlatformNodeDelegateWindows( std::weak_ptr bridge, FlutterWindowsView* view) : bridge_(bridge), view_(view) { - assert(!bridge_.expired()); - assert(view_); + FML_DCHECK(!bridge_.expired()) + << "Expired AccessibilityBridge passed to node delegate"; + FML_DCHECK(view_); } FlutterPlatformNodeDelegateWindows::~FlutterPlatformNodeDelegateWindows() { @@ -33,14 +35,15 @@ void FlutterPlatformNodeDelegateWindows::Init(std::weak_ptr bridge, ui::AXNode* node) { FlutterPlatformNodeDelegate::Init(bridge, node); ax_platform_node_ = ui::AXPlatformNode::Create(this); - assert(ax_platform_node_); + FML_DCHECK(ax_platform_node_) << "Failed to create AXPlatformNode"; } // |ui::AXPlatformNodeDelegate| gfx::NativeViewAccessible FlutterPlatformNodeDelegateWindows::GetNativeViewAccessible() { - assert(ax_platform_node_); - return ax_platform_node_->GetNativeViewAccessible(); + FML_DCHECK(ax_platform_node_) + << "AXPlatformNode hasn't been created; + return ax_platform_node_->GetNativeViewAccessible(); } // |ui::AXPlatformNodeDelegate| @@ -58,12 +61,13 @@ gfx::NativeViewAccessible FlutterPlatformNodeDelegateWindows::HitTestSync( // If any child in this node's subtree contains the point, return that child. auto bridge = bridge_.lock(); - assert(bridge); + FML_DCHECK(bridge); for (const ui::AXNode* child : GetAXNode()->children()) { std::shared_ptr win_delegate = std::static_pointer_cast( bridge->GetFlutterPlatformNodeDelegateFromID(child->id()).lock()); - assert(win_delegate); + FML_DCHECK(win_delegate) + << "No FlutterPlatformNodeDelegate found for node " << child->id(); auto hit_view = win_delegate->HitTestSync(screen_physical_pixel_x, screen_physical_pixel_y); if (hit_view) { diff --git a/shell/platform/windows/flutter_windows.cc b/shell/platform/windows/flutter_windows.cc index ad8754b2ad69d..aa8b68a381716 100644 --- a/shell/platform/windows/flutter_windows.cc +++ b/shell/platform/windows/flutter_windows.cc @@ -7,7 +7,6 @@ #include #include -#include #include #include #include diff --git a/shell/platform/windows/keyboard_key_embedder_handler.cc b/shell/platform/windows/keyboard_key_embedder_handler.cc index f31a5caa7f540..b0d9701a2126c 100644 --- a/shell/platform/windows/keyboard_key_embedder_handler.cc +++ b/shell/platform/windows/keyboard_key_embedder_handler.cc @@ -4,13 +4,13 @@ #include "flutter/shell/platform/windows/keyboard_key_embedder_handler.h" -#include #include #include #include #include +#include "flutter/fml/logging.h" #include "flutter/shell/platform/windows/keyboard_utils.h" namespace flutter { @@ -38,7 +38,7 @@ char _GetBit(char32_t ch, size_t start, size_t end) { std::string ConvertChar32ToUtf8(char32_t ch) { std::string result; - assert(0 <= ch && ch <= 0x10FFFF); + FML_DCHECK(0 <= ch && ch <= 0x10FFFF) << "Character out of range"; if (ch <= 0x007F) { result.push_back(ch); } else if (ch <= 0x07FF) { @@ -160,8 +160,8 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl( std::function callback) { const uint64_t physical_key = GetPhysicalKey(scancode, extended); const uint64_t logical_key = GetLogicalKey(key, extended, scancode); - assert(action == WM_KEYDOWN || action == WM_KEYUP || - action == WM_SYSKEYDOWN || action == WM_SYSKEYUP); + FML_DCHECK(action == WM_KEYDOWN || action == WM_KEYUP || + action == WM_SYSKEYDOWN || action == WM_SYSKEYUP); auto last_logical_record_iter = pressingRecords_.find(physical_key); bool had_record = last_logical_record_iter != pressingRecords_.end(); @@ -230,7 +230,7 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl( if (was_down) { // A normal repeated key. type = kFlutterKeyEventTypeRepeat; - assert(had_record); + FML_DCHECK(had_record); ConvertUtf32ToUtf8_(character_bytes, character); eventual_logical_record = last_logical_record; result_logical_key = last_logical_record; @@ -245,7 +245,7 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl( } else { // A normal down event (whether the system event is a repeat or not). type = kFlutterKeyEventTypeDown; - assert(!had_record); + FML_DCHECK(!had_record); ConvertUtf32ToUtf8_(character_bytes, character); eventual_logical_record = logical_key; result_logical_key = logical_key; @@ -260,7 +260,7 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl( } else { // A normal up event. type = kFlutterKeyEventTypeUp; - assert(had_record); + FML_DCHECK(had_record); // Up events never have character. character_bytes[0] = '\0'; eventual_logical_record = 0; @@ -278,7 +278,7 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl( if (record_iter != pressingRecords_.end()) { pressingRecords_.erase(record_iter); } else { - assert(false); + FML_DCHECK(false); } } @@ -375,7 +375,7 @@ void KeyboardKeyEmbedderHandler::SynchronizeCriticalToggledStates( // Never seen this key. continue; } - assert(key_info.logical_key != 0); + FML_DCHECK(key_info.logical_key != 0); // Check toggling state first, because it might alter pressing state. if (key_info.check_toggled) { @@ -440,7 +440,7 @@ void KeyboardKeyEmbedderHandler::SynchronizeCriticalPressedStates( // Never seen this key. continue; } - assert(key_info.logical_key != 0); + FML_DCHECK(key_info.logical_key != 0); if (key_info.check_pressed) { SHORT true_pressed = get_key_state_(virtual_key) & kStateMaskPressed; auto pressing_record_iter = pressingRecords_.find(key_info.physical_key); diff --git a/shell/platform/windows/keyboard_key_handler.cc b/shell/platform/windows/keyboard_key_handler.cc index 0bd71e0e96dac..264006bf6d6c7 100644 --- a/shell/platform/windows/keyboard_key_handler.cc +++ b/shell/platform/windows/keyboard_key_handler.cc @@ -83,7 +83,8 @@ void KeyboardKeyHandler::ResolvePendingEvent(uint64_t sequence_id, PendingEvent& event = **iter; event.any_handled = event.any_handled || handled; event.unreplied -= 1; - assert(event.unreplied >= 0); + FML_DCHECK(event.unreplied >= 0) + << "Pending events must have unreplied count > 0"; // If all delegates have replied, report if any of them handled the event. if (event.unreplied == 0) { std::unique_ptr event_ptr = std::move(*iter); @@ -95,7 +96,7 @@ void KeyboardKeyHandler::ResolvePendingEvent(uint64_t sequence_id, } } // The pending event should always be found. - assert(false); + FML_LOG(FATAL) << "No unreplied pending events found"; } } // namespace flutter diff --git a/shell/platform/windows/keyboard_manager.cc b/shell/platform/windows/keyboard_manager.cc index d8891a5a9c996..21433aa0e56b3 100644 --- a/shell/platform/windows/keyboard_manager.cc +++ b/shell/platform/windows/keyboard_manager.cc @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include #include #include @@ -323,7 +322,8 @@ bool KeyboardManager::HandleMessage(UINT const action, return !IsSysAction(action); } default: - assert(false); + FML_LOG(FATAL) << "No event handler for keyboard event with action " + << action; } return false; } @@ -336,7 +336,7 @@ void KeyboardManager::ProcessNextEvent() { auto pending_event = std::move(pending_events_.front()); pending_events_.pop_front(); PerformProcessEvent(std::move(pending_event), [this] { - assert(processing_event_); + FML_DCHECK(processing_event_); processing_event_ = false; ProcessNextEvent(); }); @@ -392,7 +392,7 @@ void KeyboardManager::DispatchText(const PendingEvent& event) { // keys defined by `IsPrintable` are certain characters at lower ASCII range. // These ASCII control characters are sent as WM_CHAR events for all control // key shortcuts. - assert(!event.session.empty()); + FML_DCHECK(!event.session.empty()); bool is_printable = IsPrintable(event.session.back().wparam); bool valid = event.character != 0 && is_printable; if (valid) { diff --git a/shell/platform/windows/keyboard_unittests.cc b/shell/platform/windows/keyboard_unittests.cc index 1a2f4949c83d7..30aeb3192a766 100644 --- a/shell/platform/windows/keyboard_unittests.cc +++ b/shell/platform/windows/keyboard_unittests.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "flutter/fml/logging.h" #include "flutter/shell/platform/common/json_message_codec.h" #include "flutter/shell/platform/embedder/embedder.h" #include "flutter/shell/platform/embedder/test_utils/key_codes.g.h" @@ -103,7 +104,8 @@ class TestKeyboardManager : public KeyboardManager { protected: void RedispatchEvent(std::unique_ptr event) override { - assert(!during_redispatch_); + FML_DCHECK(!during_redispatch_) + << "RedispatchEvent called while already redispatching an event"; during_redispatch_ = true; KeyboardManager::RedispatchEvent(std::move(event)); during_redispatch_ = false; @@ -235,7 +237,7 @@ class MockKeyboardManagerDelegate : public KeyboardManager::WindowDelegate, break; } default: - assert(false); + FML_LOG(FATAL) << "Unhandled KeyboardChange type " << change.type; } } } @@ -337,7 +339,7 @@ class TestFlutterWindowsView : public FlutterWindowsView { const char* args) { rapidjson::Document args_doc; args_doc.Parse(args); - assert(!args_doc.HasParseError()); + FML_DCHECK(!args_doc.HasParseError()); rapidjson::Document message_doc(rapidjson::kObjectType); auto& allocator = message_doc.GetAllocator(); @@ -473,7 +475,7 @@ class KeyboardTester { } void InjectKeyboardChanges(std::vector changes) { - assert(window_ != nullptr); + FML_DCHECK(window_ != nullptr); window_->InjectKeyboardChanges(changes); } diff --git a/shell/platform/windows/keyboard_utils.cc b/shell/platform/windows/keyboard_utils.cc index 030123b8351f6..aafdb0b3361ad 100644 --- a/shell/platform/windows/keyboard_utils.cc +++ b/shell/platform/windows/keyboard_utils.cc @@ -4,14 +4,16 @@ #include "keyboard_utils.h" +#include "flutter/fml/logging.h" + namespace flutter { std::u16string EncodeUtf16(char32_t character) { // Algorithm: https://en.wikipedia.org/wiki/UTF-16#Description std::u16string result; // Invalid value. - assert(!(character >= 0xD800 && character <= 0xDFFF) && - !(character > 0x10FFFF)); + FML_DCHECK(!(character >= 0xD800 && character <= 0xDFFF) && + !(character > 0x10FFFF)); if ((character >= 0xD800 && character <= 0xDFFF) || (character > 0x10FFFF)) { return result; } diff --git a/shell/platform/windows/keyboard_utils.h b/shell/platform/windows/keyboard_utils.h index f71e12f5ebb5e..e5261e3ad84a7 100644 --- a/shell/platform/windows/keyboard_utils.h +++ b/shell/platform/windows/keyboard_utils.h @@ -5,8 +5,8 @@ #ifndef FLUTTER_SHELL_PLATFORM_WINDOWS_KEYBOARD_WIN32_COMMON_H_ #define FLUTTER_SHELL_PLATFORM_WINDOWS_KEYBOARD_WIN32_COMMON_H_ -#include #include + #include namespace flutter { diff --git a/shell/platform/windows/platform_handler.cc b/shell/platform/windows/platform_handler.cc index ff1e02d564bb0..b7e93e3651ab6 100644 --- a/shell/platform/windows/platform_handler.cc +++ b/shell/platform/windows/platform_handler.cc @@ -159,7 +159,7 @@ bool ScopedClipboard::HasString() { } std::variant ScopedClipboard::GetString() { - assert(opened_); + FML_DCHECK(opened_) << "Called GetString when clipboard is closed"; HANDLE data = ::GetClipboardData(CF_UNICODETEXT); if (data == nullptr) { @@ -174,7 +174,7 @@ std::variant ScopedClipboard::GetString() { } int ScopedClipboard::SetString(const std::wstring string) { - assert(opened_); + FML_DCHECK(opened_) << "Called GetString when clipboard is closed"; if (!::EmptyClipboard()) { return ::GetLastError(); } diff --git a/shell/platform/windows/testing/test_binary_messenger.h b/shell/platform/windows/testing/test_binary_messenger.h index 3cb38e89c1a7c..b311c3ae171a6 100644 --- a/shell/platform/windows/testing/test_binary_messenger.h +++ b/shell/platform/windows/testing/test_binary_messenger.h @@ -5,11 +5,11 @@ #ifndef FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_TEST_BINARY_MESSENGER_H_ #define FLUTTER_SHELL_PLATFORM_WINDOWS_TESTING_TEST_BINARY_MESSENGER_H_ -#include #include #include #include +#include "flutter/fml/logging.h" #include "flutter/shell/platform/common/client_wrapper/include/flutter/binary_messenger.h" namespace flutter { @@ -53,7 +53,7 @@ class TestBinaryMessenger : public BinaryMessenger { size_t message_size, BinaryReply reply) const override { // If something under test sends a message, the test should be handling it. - assert(send_handler_); + FML_DCHECK(send_handler_); send_handler_(channel, message, message_size, reply); } diff --git a/shell/platform/windows/testing/test_keyboard.cc b/shell/platform/windows/testing/test_keyboard.cc index d752f027c80c0..4d2109a657108 100644 --- a/shell/platform/windows/testing/test_keyboard.cc +++ b/shell/platform/windows/testing/test_keyboard.cc @@ -3,11 +3,13 @@ // found in the LICENSE file. #include "flutter/shell/platform/windows/testing/test_keyboard.h" -#include "flutter/shell/platform/common/json_message_codec.h" -#include "flutter/shell/platform/embedder/test_utils/proc_table_replacement.h" #include +#include "flutter/fml/logging.h" +#include "flutter/shell/platform/common/json_message_codec.h" +#include "flutter/shell/platform/embedder/test_utils/proc_table_replacement.h" + namespace flutter { namespace testing { @@ -205,7 +207,8 @@ void MockMessageQueue::PushBack(const Win32Message* message) { } LRESULT MockMessageQueue::DispatchFront() { - assert(!_pending_messages.empty()); + FML_DCHECK(!_pending_messages.empty()) + << "Called DispatchFront while pending message queue is empty"; Win32Message message = _pending_messages.front(); _pending_messages.pop_front(); _sent_messages.push_back(message); diff --git a/shell/platform/windows/text_input_manager.cc b/shell/platform/windows/text_input_manager.cc index 2bdc8fb13f4a0..a270abcc987ce 100644 --- a/shell/platform/windows/text_input_manager.cc +++ b/shell/platform/windows/text_input_manager.cc @@ -2,17 +2,14 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - #include "flutter/shell/platform/windows/text_input_manager.h" #include -#include #include +#include "flutter/fml/logging.h" + namespace flutter { // RAII wrapper for the Win32 Input Method Manager context. @@ -21,7 +18,7 @@ class ImmContext { ImmContext(HWND window_handle) : context_(::ImmGetContext(window_handle)), window_handle_(window_handle) { - assert(window_handle); + FML_DCHECK(window_handle); } ~ImmContext() { @@ -39,7 +36,7 @@ class ImmContext { // Returns the IMM context. HIMC get() { - assert(context_); + FML_DCHECK(context_); return context_; }