Skip to content

Commit

Permalink
[windows] Use FML_DCHECK in place of C assert (flutter#38826)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
cbracken authored Jan 13, 2023
1 parent 99509a7 commit 091c785
Show file tree
Hide file tree
Showing 13 changed files with 57 additions and 47 deletions.
8 changes: 5 additions & 3 deletions shell/platform/windows/accessibility_bridge_windows.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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(
Expand All @@ -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<FlutterPlatformNodeDelegateWindows> win_delegate =
std::static_pointer_cast<FlutterPlatformNodeDelegateWindows>(
node_delegate);
Expand Down
15 changes: 9 additions & 6 deletions shell/platform/windows/flutter_platform_node_delegate_windows.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -18,8 +19,9 @@ FlutterPlatformNodeDelegateWindows::FlutterPlatformNodeDelegateWindows(
std::weak_ptr<AccessibilityBridge> 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() {
Expand All @@ -33,13 +35,13 @@ void FlutterPlatformNodeDelegateWindows::Init(std::weak_ptr<OwnerBridge> 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_);
FML_DCHECK(ax_platform_node_) << "AXPlatformNode hasn't been created";
return ax_platform_node_->GetNativeViewAccessible();
}

Expand All @@ -58,12 +60,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<FlutterPlatformNodeDelegateWindows> win_delegate =
std::static_pointer_cast<FlutterPlatformNodeDelegateWindows>(
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) {
Expand Down
1 change: 0 additions & 1 deletion shell/platform/windows/flutter_windows.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include <io.h>

#include <algorithm>
#include <cassert>
#include <chrono>
#include <cstdlib>
#include <filesystem>
Expand Down
20 changes: 10 additions & 10 deletions shell/platform/windows/keyboard_key_embedder_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@

#include "flutter/shell/platform/windows/keyboard_key_embedder_handler.h"

#include <assert.h>
#include <windows.h>

#include <chrono>
#include <iostream>
#include <string>

#include "flutter/fml/logging.h"
#include "flutter/shell/platform/windows/keyboard_utils.h"

namespace flutter {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -160,8 +160,8 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl(
std::function<void(bool)> 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();
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -278,7 +278,7 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl(
if (record_iter != pressingRecords_.end()) {
pressingRecords_.erase(record_iter);
} else {
assert(false);
FML_DCHECK(false);
}
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 4 additions & 2 deletions shell/platform/windows/keyboard_key_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<PendingEvent> event_ptr = std::move(*iter);
Expand All @@ -95,7 +96,8 @@ void KeyboardKeyHandler::ResolvePendingEvent(uint64_t sequence_id,
}
}
// The pending event should always be found.
assert(false);
FML_LOG(FATAL) << "Could not find pending key event for sequence ID "
<< sequence_id;
}

} // namespace flutter
8 changes: 4 additions & 4 deletions shell/platform/windows/keyboard_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 <assert.h>
#include <memory>
#include <string>

Expand Down Expand Up @@ -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;
}
Expand All @@ -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();
});
Expand Down Expand Up @@ -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) {
Expand Down
10 changes: 6 additions & 4 deletions shell/platform/windows/keyboard_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -103,7 +104,8 @@ class TestKeyboardManager : public KeyboardManager {

protected:
void RedispatchEvent(std::unique_ptr<PendingEvent> 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;
Expand Down Expand Up @@ -235,7 +237,7 @@ class MockKeyboardManagerDelegate : public KeyboardManager::WindowDelegate,
break;
}
default:
assert(false);
FML_LOG(FATAL) << "Unhandled KeyboardChange type " << change.type;
}
}
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -473,7 +475,7 @@ class KeyboardTester {
}

void InjectKeyboardChanges(std::vector<KeyboardChange> changes) {
assert(window_ != nullptr);
FML_DCHECK(window_ != nullptr);
window_->InjectKeyboardChanges(changes);
}

Expand Down
6 changes: 4 additions & 2 deletions shell/platform/windows/keyboard_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion shell/platform/windows/keyboard_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
#ifndef FLUTTER_SHELL_PLATFORM_WINDOWS_KEYBOARD_WIN32_COMMON_H_
#define FLUTTER_SHELL_PLATFORM_WINDOWS_KEYBOARD_WIN32_COMMON_H_

#include <assert.h>
#include <stdint.h>

#include <string>

namespace flutter {
Expand Down
4 changes: 2 additions & 2 deletions shell/platform/windows/platform_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ bool ScopedClipboard::HasString() {
}

std::variant<std::wstring, int> ScopedClipboard::GetString() {
assert(opened_);
FML_DCHECK(opened_) << "Called GetString when clipboard is closed";

HANDLE data = ::GetClipboardData(CF_UNICODETEXT);
if (data == nullptr) {
Expand All @@ -174,7 +174,7 @@ std::variant<std::wstring, int> ScopedClipboard::GetString() {
}

int ScopedClipboard::SetString(const std::wstring string) {
assert(opened_);
FML_DCHECK(opened_) << "Called GetString when clipboard is closed";
if (!::EmptyClipboard()) {
return ::GetLastError();
}
Expand Down
4 changes: 2 additions & 2 deletions shell/platform/windows/testing/test_binary_messenger.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <cassert>
#include <functional>
#include <map>
#include <string>

#include "flutter/fml/logging.h"
#include "flutter/shell/platform/common/client_wrapper/include/flutter/binary_messenger.h"

namespace flutter {
Expand Down Expand Up @@ -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);
}

Expand Down
9 changes: 6 additions & 3 deletions shell/platform/windows/testing/test_keyboard.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 <rapidjson/document.h>

#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 {

Expand Down Expand Up @@ -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);
Expand Down
Loading

0 comments on commit 091c785

Please sign in to comment.