Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: inline simple getters, pt . 2 #41254

Merged
merged 8 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion shell/browser/api/electron_api_app.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1681,7 +1681,7 @@ gin::ObjectTemplateBuilder App::GetObjectTemplateBuilder(v8::Isolate* isolate) {
.SetMethod("setBadgeCount",
base::BindRepeating(&Browser::SetBadgeCount, browser))
.SetMethod("getBadgeCount",
base::BindRepeating(&Browser::GetBadgeCount, browser))
base::BindRepeating(&Browser::badge_count, browser))
.SetMethod("getLoginItemSettings", &App::GetLoginItemSettings)
.SetMethod("setLoginItemSettings",
base::BindRepeating(&Browser::SetLoginItemSettings, browser))
Expand Down
4 changes: 2 additions & 2 deletions shell/browser/api/electron_api_safe_storage.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ bool IsEncryptionAvailable() {
return OSCrypt::IsEncryptionAvailable() ||
(use_password_v10 &&
static_cast<BrowserProcessImpl*>(g_browser_process)
->GetLinuxStorageBackend() == "basic_text");
->linux_storage_backend() == "basic_text");
#else
return OSCrypt::IsEncryptionAvailable();
#endif
Expand All @@ -46,7 +46,7 @@ std::string GetSelectedLinuxBackend() {
if (!Browser::Get()->is_ready())
return "unknown";
return static_cast<BrowserProcessImpl*>(g_browser_process)
->GetLinuxStorageBackend();
->linux_storage_backend();
}
#endif

Expand Down
30 changes: 11 additions & 19 deletions shell/browser/api/electron_api_web_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -852,7 +852,7 @@ WebContents::WebContents(v8::Isolate* isolate,
session_.Reset(isolate, session.ToV8());

std::unique_ptr<content::WebContents> web_contents;
if (IsGuest()) {
if (is_guest()) {
scoped_refptr<content::SiteInstance> site_instance =
content::SiteInstance::CreateForURL(session->browser_context(),
GURL("chrome-guest://fake-host"));
Expand Down Expand Up @@ -922,7 +922,7 @@ void WebContents::InitWithSessionAndOptions(
const gin_helper::Dictionary& options) {
Observe(owned_web_contents.get());
InitWithWebContents(std::move(owned_web_contents), session->browser_context(),
IsGuest());
is_guest());

inspectable_web_contents_->GetView()->SetDelegate(this);

Expand Down Expand Up @@ -982,7 +982,7 @@ void WebContents::InitWithSessionAndOptions(

SetUserAgent(GetBrowserContext()->GetUserAgent());

if (IsGuest()) {
if (is_guest()) {
NativeWindow* owner_window = nullptr;
if (embedder_) {
// New WebContents's owner_window is the embedder's owner_window.
Expand Down Expand Up @@ -1017,7 +1017,7 @@ void WebContents::InitWithExtensionView(v8::Isolate* isolate,
// Allow toggling DevTools for background pages
Observe(web_contents);
InitWithWebContents(std::unique_ptr<content::WebContents>(web_contents),
GetBrowserContext(), IsGuest());
GetBrowserContext(), is_guest());
inspectable_web_contents_->GetView()->SetDelegate(this);
}
#endif
Expand Down Expand Up @@ -1066,7 +1066,7 @@ WebContents::~WebContents() {

// For guest view based on OOPIF, the WebContents is released by the embedder
// frame, and we need to clear the reference to the memory.
bool not_owned_by_this = IsGuest() && attached_;
bool not_owned_by_this = is_guest() && attached_;
#if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
// And background pages are owned by extensions::ExtensionHost.
if (type_ == Type::kBackgroundPage)
Expand Down Expand Up @@ -1096,7 +1096,7 @@ void WebContents::DeleteThisIfAlive() {
void WebContents::Destroy() {
// The content::WebContents should be destroyed asynchronously when possible
// as user may choose to destroy WebContents during an event of it.
if (Browser::Get()->is_shutting_down() || IsGuest()) {
if (Browser::Get()->is_shutting_down() || is_guest()) {
DeleteThisIfAlive();
} else {
content::GetUIThreadTaskRunner({})->PostTask(
Expand Down Expand Up @@ -2155,7 +2155,7 @@ void WebContents::DidFinishNavigation(
Emit("did-navigate", url, http_response_code, http_status_text);
}
}
if (IsGuest())
if (is_guest())
Emit("load-commit", url, is_main_frame);
} else {
auto url = navigation_handle->GetURL();
Expand Down Expand Up @@ -2374,10 +2374,6 @@ base::ProcessId WebContents::GetOSProcessID() const {
return base::GetProcId(process_handle);
}

WebContents::Type WebContents::GetType() const {
return type_;
}

bool WebContents::Equal(const WebContents* web_contents) const {
return ID() == web_contents->ID();
}
Expand Down Expand Up @@ -3371,7 +3367,7 @@ bool WebContents::IsFocused() const {
if (!view)
return false;

if (GetType() != Type::kBackgroundPage) {
if (type() != Type::kBackgroundPage) {
auto* window = web_contents()->GetNativeView()->GetToplevelWindow();
if (window && !window->IsVisible())
return false;
Expand Down Expand Up @@ -3567,10 +3563,6 @@ void WebContents::OnCursorChanged(const ui::Cursor& cursor) {
}
}

bool WebContents::IsGuest() const {
return type_ == Type::kWebView;
}

void WebContents::AttachToIframe(content::WebContents* embedder_web_contents,
int embedder_frame_id) {
attached_ = true;
Expand Down Expand Up @@ -3782,7 +3774,7 @@ void WebContents::SetImageAnimationPolicy(const std::string& new_policy) {
}

void WebContents::SetBackgroundColor(std::optional<SkColor> maybe_color) {
SkColor color = maybe_color.value_or((IsGuest() && guest_transparent_) ||
SkColor color = maybe_color.value_or((is_guest() && guest_transparent_) ||
type_ == Type::kBrowserView
? SK_ColorTRANSPARENT
: SK_ColorWHITE);
Expand Down Expand Up @@ -4304,7 +4296,7 @@ void WebContents::UpdateHtmlApiFullscreen(bool fullscreen) {
}

// Make sure all child webviews quit html fullscreen.
if (!fullscreen && !IsGuest()) {
if (!fullscreen && !is_guest()) {
auto* manager = WebViewManager::GetWebViewManager(web_contents());
manager->ForEachGuest(web_contents(), [&](content::WebContents* guest) {
WebContents* api_web_contents = WebContents::From(guest);
Expand Down Expand Up @@ -4416,7 +4408,7 @@ void WebContents::FillObjectTemplate(v8::Isolate* isolate,
.SetMethod("getZoomLevel", &WebContents::GetZoomLevel)
.SetMethod("setZoomFactor", &WebContents::SetZoomFactor)
.SetMethod("getZoomFactor", &WebContents::GetZoomFactor)
.SetMethod("getType", &WebContents::GetType)
.SetMethod("getType", &WebContents::type)
.SetMethod("_getPreloadPaths", &WebContents::GetPreloadPaths)
.SetMethod("getLastWebPreferences", &WebContents::GetLastWebPreferences)
.SetMethod("getOwnerBrowserWindow", &WebContents::GetOwnerBrowserWindow)
Expand Down
4 changes: 2 additions & 2 deletions shell/browser/api/electron_api_web_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ class WebContents : public ExclusiveAccessContext,
void SetBackgroundThrottling(bool allowed);
int GetProcessID() const;
base::ProcessId GetOSProcessID() const;
Type GetType() const;
[[nodiscard]] Type type() const { return type_; }
bool Equal(const WebContents* web_contents) const;
void LoadURL(const GURL& url, const gin_helper::Dictionary& options);
void Reload();
Expand Down Expand Up @@ -292,7 +292,7 @@ class WebContents : public ExclusiveAccessContext,
v8::Local<v8::Promise> CapturePage(gin::Arguments* args);

// Methods for creating <webview>.
bool IsGuest() const;
[[nodiscard]] bool is_guest() const { return type_ == Type::kWebView; }
void AttachToIframe(content::WebContents* embedder_web_contents,
int embedder_frame_id);
void DetachFromOuterFrame();
Expand Down
2 changes: 1 addition & 1 deletion shell/browser/api/electron_api_web_contents_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ - (void)redispatchKeyEvent:(NSEvent*)event;
if (!view)
return false;

if (GetType() != Type::kBackgroundPage) {
if (type() != Type::kBackgroundPage) {
auto window = [web_contents()->GetNativeView().GetNativeNSView() window];
// On Mac the render widget host view does not lose focus when the window
// loses focus so check if the top level window is the key window.
Expand Down
4 changes: 0 additions & 4 deletions shell/browser/browser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,6 @@ void Browser::SetName(const std::string& name) {
OverriddenApplicationName() = name;
}

int Browser::GetBadgeCount() {
return badge_count_;
}

bool Browser::OpenFile(const std::string& file_path) {
bool prevent_default = false;
for (BrowserObserver& observer : observers_)
Expand Down
2 changes: 1 addition & 1 deletion shell/browser/browser.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class Browser : public WindowListObserver {

// Set/Get the badge count.
bool SetBadgeCount(std::optional<int> count);
int GetBadgeCount();
[[nodiscard]] int badge_count() const { return badge_count_; }

#if BUILDFLAG(IS_WIN)
struct LaunchItem {
Expand Down
4 changes: 0 additions & 4 deletions shell/browser/browser_process_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -340,10 +340,6 @@ void BrowserProcessImpl::SetLinuxStorageBackend(
break;
}
}

const std::string& BrowserProcessImpl::GetLinuxStorageBackend() const {
return selected_linux_storage_backend_;
}
#endif // BUILDFLAG(IS_LINUX)

void BrowserProcessImpl::SetApplicationLocale(const std::string& locale) {
Expand Down
4 changes: 3 additions & 1 deletion shell/browser/browser_process_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ class BrowserProcessImpl : public BrowserProcess {

#if BUILDFLAG(IS_LINUX)
void SetLinuxStorageBackend(os_crypt::SelectedLinuxBackend selected_backend);
const std::string& GetLinuxStorageBackend() const;
[[nodiscard]] const std::string& linux_storage_backend() const {
return selected_linux_storage_backend_;
}
#endif

void EndSession() override {}
Expand Down
8 changes: 0 additions & 8 deletions shell/browser/native_window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -518,22 +518,14 @@

bool NativeWindow::IsMenuBarVisible() const {
return true;
}

double NativeWindow::GetAspectRatio() const {
return aspect_ratio_;
}

gfx::Size NativeWindow::GetAspectRatioExtraSize() const {
return aspect_ratio_extraSize_;
}

void NativeWindow::SetAspectRatio(double aspect_ratio,
const gfx::Size& extra_size) {
aspect_ratio_ = aspect_ratio;
aspect_ratio_extraSize_ = extra_size;
}

Check failure on line 528 in shell/browser/native_window.cc

View check run for this annotation

trop / Backportable? - 28-x-y

shell/browser/native_window.cc#L521-L528

Patch Conflict
Raw output
++<<<<<<< HEAD
 +double NativeWindow::GetAspectRatio() {
 +  return aspect_ratio_;
 +}
 +
 +gfx::Size NativeWindow::GetAspectRatioExtraSize() {
 +  return aspect_ratio_extraSize_;
 +}
 +
++=======
++>>>>>>> refactor: inline NativeWindow::aspect_ratio()

Check failure on line 528 in shell/browser/native_window.cc

View check run for this annotation

trop / Backportable? - 28-x-y

shell/browser/native_window.cc#L521-L528

Patch Conflict
Raw output
++<<<<<<< HEAD
 +double NativeWindow::GetAspectRatio() {
 +  return aspect_ratio_;
 +}
 +
 +gfx::Size NativeWindow::GetAspectRatioExtraSize() {
 +  return aspect_ratio_extraSize_;
 +}
 +
++=======
++>>>>>>> refactor: inline NativeWindow::aspect_ratio()
void NativeWindow::PreviewFile(const std::string& path,
const std::string& display_name) {}

Expand Down
6 changes: 4 additions & 2 deletions shell/browser/native_window.h
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,11 @@
virtual bool IsMenuBarVisible() const;

// Set the aspect ratio when resizing window.
double GetAspectRatio() const;
gfx::Size GetAspectRatioExtraSize() const;
[[nodiscard]] double aspect_ratio() const { return aspect_ratio_; }
[[nodiscard]] gfx::Size aspect_ratio_extra_size() const {
return aspect_ratio_extraSize_;
}
virtual void SetAspectRatio(double aspect_ratio, const gfx::Size& extra_size);

Check failure on line 271 in shell/browser/native_window.h

View check run for this annotation

trop / Backportable? - 28-x-y

shell/browser/native_window.h#L270-L271

Patch Conflict
Raw output
++<<<<<<< HEAD
 +  double GetAspectRatio();
 +  gfx::Size GetAspectRatioExtraSize();
++=======
+   [[nodiscard]] double aspect_ratio() const { return aspect_ratio_; }
+   [[nodiscard]] gfx::Size aspect_ratio_extra_size() const {
+     return aspect_ratio_extraSize_;
+   }
++>>>>>>> refactor: inline NativeWindow::aspect_ratio()

Check failure on line 271 in shell/browser/native_window.h

View check run for this annotation

trop / Backportable? - 28-x-y

shell/browser/native_window.h#L270-L271

Patch Conflict
Raw output
++<<<<<<< HEAD
 +  double GetAspectRatio();
 +  gfx::Size GetAspectRatioExtraSize();
++=======
+   [[nodiscard]] double aspect_ratio() const { return aspect_ratio_; }
+   [[nodiscard]] gfx::Size aspect_ratio_extra_size() const {
+     return aspect_ratio_extraSize_;
+   }
++>>>>>>> refactor: inline NativeWindow::aspect_ratio()

// File preview APIs.
virtual void PreviewFile(const std::string& path,
Expand Down
2 changes: 1 addition & 1 deletion shell/browser/native_window_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ void ReorderChildWindowAbove(NSWindow* child_window, NSWindow* other_window) {
if (HasStyleMask(NSWindowStyleMaskResizable) != 0)
return [window_ isZoomed];

NSRect rectScreen = GetAspectRatio() > 0.0
NSRect rectScreen = aspect_ratio() > 0.0
? default_frame_for_zoom()
: [[NSScreen mainScreen] visibleFrame];

Expand Down
23 changes: 5 additions & 18 deletions shell/browser/ui/autofill_popup.cc
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ void AutofillPopup::SetItems(const std::vector<std::u16string>& values,
void AutofillPopup::AcceptSuggestion(int index) {
mojo::AssociatedRemote<mojom::ElectronAutofillAgent> autofill_agent;
frame_host_->GetRemoteAssociatedInterfaces()->GetInterface(&autofill_agent);
autofill_agent->AcceptDataListSuggestion(GetValueAt(index));
autofill_agent->AcceptDataListSuggestion(value_at(index));
}

void AutofillPopup::UpdatePopupBounds() {
Expand Down Expand Up @@ -272,11 +272,10 @@ int AutofillPopup::GetDesiredPopupWidth() {
int popup_width = element_bounds_.width();

for (size_t i = 0; i < values_.size(); ++i) {
int row_size =
kEndPadding + 2 * kPopupBorderThickness +
gfx::GetStringWidth(GetValueAt(i), GetValueFontListForRow(i)) +
gfx::GetStringWidth(GetLabelAt(i), GetLabelFontListForRow(i));
if (!GetLabelAt(i).empty())
int row_size = kEndPadding + 2 * kPopupBorderThickness +
gfx::GetStringWidth(value_at(i), GetValueFontListForRow(i)) +
gfx::GetStringWidth(label_at(i), GetLabelFontListForRow(i));
if (!label_at(i).empty())
row_size += kNamePadding + kEndPadding;

popup_width = std::max(popup_width, row_size);
Expand Down Expand Up @@ -307,18 +306,6 @@ ui::ColorId AutofillPopup::GetBackgroundColorIDForRow(int index) const {
: ui::kColorResultsTableNormalBackground;
}

int AutofillPopup::GetLineCount() {
return values_.size();
}

std::u16string AutofillPopup::GetValueAt(int i) {
return values_.at(i);
}

std::u16string AutofillPopup::GetLabelAt(int i) {
return labels_.at(i);
}

int AutofillPopup::LineFromY(int y) const {
int current_height = kPopupBorderThickness;

Expand Down
6 changes: 3 additions & 3 deletions shell/browser/ui/autofill_popup.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ class AutofillPopup : public views::ViewObserver {
const gfx::FontList& GetLabelFontListForRow(int index) const;
ui::ColorId GetBackgroundColorIDForRow(int index) const;

int GetLineCount();
std::u16string GetValueAt(int i);
std::u16string GetLabelAt(int i);
int line_count() const { return values_.size(); }
const std::u16string& value_at(int i) const { return values_.at(i); }
const std::u16string& label_at(int i) const { return labels_.at(i); }
int LineFromY(int y) const;

int selected_index_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ - (instancetype)initWithInspectableWebContentsViewMac:
devtools_is_first_responder_ = NO;
attached_to_window_ = NO;

if (inspectableWebContentsView_->inspectable_web_contents()->IsGuest()) {
if (inspectableWebContentsView_->inspectable_web_contents()->is_guest()) {
fake_view_ = [[NSView alloc] init];
[fake_view_ setAutoresizingMask:NSViewWidthSizable | NSViewHeightSizable];
[self addSubview:fake_view_];
Expand Down
4 changes: 2 additions & 2 deletions shell/browser/ui/cocoa/electron_menu_controller.mm
Original file line number Diff line number Diff line change
Expand Up @@ -493,8 +493,8 @@ - (NSMenu*)menu {
if (menu_)
return menu_;

if (model_ && model_->GetSharingItem()) {
NSMenu* menu = [self createShareMenuForItem:*model_->GetSharingItem()];
if (model_ && model_->sharing_item()) {
NSMenu* menu = [self createShareMenuForItem:*model_->sharing_item()];
menu_ = menu;
} else {
menu_ = [[NSMenu alloc] initWithTitle:@""];
Expand Down
13 changes: 6 additions & 7 deletions shell/browser/ui/cocoa/electron_ns_window_delegate.mm
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ - (void)windowDidChangeOcclusionState:(NSNotification*)notification {
- (NSRect)windowWillUseStandardFrame:(NSWindow*)window
defaultFrame:(NSRect)frame {
if (!shell_->zoom_to_page_width()) {
if (shell_->GetAspectRatio() > 0.0)
if (shell_->aspect_ratio() > 0.0)
shell_->set_default_frame_for_zoom(frame);
return frame;
}
Expand Down Expand Up @@ -104,7 +104,7 @@ - (NSRect)windowWillUseStandardFrame:(NSWindow*)window
// Set the width. Don't touch y or height.
frame.size.width = zoomed_width;

if (shell_->GetAspectRatio() > 0.0)
if (shell_->aspect_ratio() > 0.0)
shell_->set_default_frame_for_zoom(frame);

return frame;
Expand Down Expand Up @@ -139,13 +139,12 @@ - (void)windowDidResignKey:(NSNotification*)notification {

- (NSSize)windowWillResize:(NSWindow*)sender toSize:(NSSize)frameSize {
NSSize newSize = frameSize;
double aspectRatio = shell_->GetAspectRatio();
NSWindow* window = shell_->GetNativeWindow().GetNativeNSWindow();

if (aspectRatio > 0.0) {
gfx::Size windowSize = shell_->GetSize();
gfx::Size contentSize = shell_->GetContentSize();
gfx::Size extraSize = shell_->GetAspectRatioExtraSize();
if (const double aspectRatio = shell_->aspect_ratio(); aspectRatio > 0.0) {
const gfx::Size windowSize = shell_->GetSize();
const gfx::Size contentSize = shell_->GetContentSize();
const gfx::Size extraSize = shell_->aspect_ratio_extra_size();

double titleBarHeight = windowSize.height() - contentSize.height();
double extraWidthPlusFrame =
Expand Down
5 changes: 0 additions & 5 deletions shell/browser/ui/electron_menu_model.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,6 @@ bool ElectronMenuModel::GetSharingItemAt(size_t index,
void ElectronMenuModel::SetSharingItem(SharingItem item) {
sharing_item_.emplace(std::move(item));
}

const std::optional<ElectronMenuModel::SharingItem>&
ElectronMenuModel::GetSharingItem() const {
return sharing_item_;
}
#endif

void ElectronMenuModel::MenuWillClose() {
Expand Down
5 changes: 4 additions & 1 deletion shell/browser/ui/electron_menu_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,10 @@ class ElectronMenuModel : public ui::SimpleMenuModel {
bool GetSharingItemAt(size_t index, SharingItem* item) const;
// Set/Get the SharingItem of this menu.
void SetSharingItem(SharingItem item);
const std::optional<SharingItem>& GetSharingItem() const;
[[nodiscard]] const std::optional<SharingItem>& sharing_item() const {
return sharing_item_;
}

#endif

// ui::SimpleMenuModel:
Expand Down