DevTools/Android: Implement Browser.setWindowBounds [chromium/src : main]
Alex Rudenko (Gerrit)
Alex Rudenko added 5 comments![]( "Open in Gerrit")
Thanks, mostly LGTM. Please wait for a review from someone more familiar with Android implementation.
ui::BaseWindow* FindWindowById(int window_id, bool* is_pending);Consider updating the signature to return a `std::pair<ui::BaseWindow*, bool>` or a custom struct to avoid the output parameter.
ui::BaseWindow* window = FindWindowById(window_id, /*is_pending=*/nullptr);This would simplify to:
```cpp
auto [window, is_pending] = FindWindowById(window_id);
```
bool* is_pending) {To avoid using an output parameter (`bool* is_pending`), you could change this method to return a `std::pair<ui::BaseWindow*, bool>` or a small struct. This aligns with the Google C++ Style Guide recommendation to prefer return values over output parameters, and allows the use of structured bindings at the call site.
For example:
```cpp
std::pair<ui::BaseWindow*, bool> BrowserHandlerAndroid::FindWindowById(int window_id) {
if (ui::BaseWindow* window = FindBrowserWindowById(window_id)) {
return {window, false};
}
auto tracked_window = tracked_browser_windows_.find(window_id);
if (tracked_window != tracked_browser_windows_.end()) {
if (tracked_window->second) {
return {tracked_window->second->GetWindow(), true};
} else {
tracked_browser_windows_.erase(tracked_window);
}
}
return {nullptr, false};
}
```
ui::BaseWindow* window = FindWindowById(window_id, &is_pending);If you change `FindWindowById` to return a `std::pair` or struct, you can use structured bindings here:
```cpp
auto [window, is_pending] = FindWindowById(window_id);
```
Related details
- Alex N. Jose
- Jack Thiesen
- Julio Piubello
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Julio Piubello (Gerrit)
Julio Piubello added 4 comments![]( "Open in Gerrit")
ui::BaseWindow* FindWindowById(int window_id, bool* is_pending);Consider updating the signature to return a `std::pair<ui::BaseWindow*, bool>` or a custom struct to avoid the output parameter.
done!
ui::BaseWindow* window = FindWindowById(window_id, /*is_pending=*/nullptr);This would simplify to:
```cpp
auto [window, is_pending] = FindWindowById(window_id);
```
done!
bool* is_pending) {To avoid using an output parameter (`bool* is_pending`), you could change this method to return a `std::pair<ui::BaseWindow*, bool>` or a small struct. This aligns with the Google C++ Style Guide recommendation to prefer return values over output parameters, and allows the use of structured bindings at the call site.
For example:
```cpp
std::pair<ui::BaseWindow*, bool> BrowserHandlerAndroid::FindWindowById(int window_id) {
if (ui::BaseWindow* window = FindBrowserWindowById(window_id)) {
return {window, false};
}auto tracked_window = tracked_browser_windows_.find(window_id);
if (tracked_window != tracked_browser_windows_.end()) {
if (tracked_window->second) {
return {tracked_window->second->GetWindow(), true};
} else {
tracked_browser_windows_.erase(tracked_window);
}
}
return {nullptr, false};
}
```
ok! changed to a WindowLookupResult struct instead
ui::BaseWindow* window = FindWindowById(window_id, &is_pending);If you change `FindWindowById` to return a `std::pair` or struct, you can use structured bindings here:
```cpp
auto [window, is_pending] = FindWindowById(window_id);
```
- Alex N. Jose
- Alex Rudenko
- Jack Thiesen
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |