From f959fb0c963701c40f309eb8256530f788d5851e Mon Sep 17 00:00:00 2001 From: Mikhail Leliakin Date: Tue, 11 Jul 2023 19:01:30 +1000 Subject: [PATCH] feat: `browserWindow.getBrowserViews()` to return sorted by z-index array (#38943) --- docs/api/browser-window.md | 4 +-- shell/browser/api/electron_api_base_window.cc | 26 ++++++++++++------- shell/browser/api/electron_api_base_window.h | 2 +- spec/api-browser-view-spec.ts | 17 ++++++++++++ 4 files changed, 36 insertions(+), 13 deletions(-) diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md index e072a2f77a98..7d927c29c33e 100644 --- a/docs/api/browser-window.md +++ b/docs/api/browser-window.md @@ -1651,8 +1651,8 @@ Throws an error if `browserView` is not attached to `win`. #### `win.getBrowserViews()` _Experimental_ -Returns `BrowserView[]` - an array of all BrowserViews that have been attached -with `addBrowserView` or `setBrowserView`. +Returns `BrowserView[]` - a sorted by z-index array of all BrowserViews that have been attached +with `addBrowserView` or `setBrowserView`. The top-most BrowserView is the last element of the array. **Note:** The BrowserView API is currently experimental and may change or be removed in future Electron releases. diff --git a/shell/browser/api/electron_api_base_window.cc b/shell/browser/api/electron_api_base_window.cc index 5c680782e43f..421417ec9b00 100644 --- a/shell/browser/api/electron_api_base_window.cc +++ b/shell/browser/api/electron_api_base_window.cc @@ -4,6 +4,7 @@ #include "shell/browser/api/electron_api_base_window.h" +#include #include #include #include @@ -756,7 +757,7 @@ void BaseWindow::SetBrowserView( } void BaseWindow::AddBrowserView(gin::Handle browser_view) { - if (!base::Contains(browser_views_, browser_view->ID())) { + if (!base::Contains(browser_views_, browser_view.ToV8())) { // If we're reparenting a BrowserView, ensure that it's detached from // its previous owner window. BaseWindow* owner_window = browser_view->owner_window(); @@ -770,17 +771,19 @@ void BaseWindow::AddBrowserView(gin::Handle browser_view) { window_->AddBrowserView(browser_view->view()); window_->AddDraggableRegionProvider(browser_view.get()); browser_view->SetOwnerWindow(this); - browser_views_[browser_view->ID()].Reset(isolate(), browser_view.ToV8()); + browser_views_.emplace_back().Reset(isolate(), browser_view.ToV8()); } } void BaseWindow::RemoveBrowserView(gin::Handle browser_view) { - auto iter = browser_views_.find(browser_view->ID()); + auto iter = std::find(browser_views_.begin(), browser_views_.end(), + browser_view.ToV8()); + if (iter != browser_views_.end()) { window_->RemoveBrowserView(browser_view->view()); window_->RemoveDraggableRegionProvider(browser_view.get()); browser_view->SetOwnerWindow(nullptr); - iter->second.Reset(); + iter->Reset(); browser_views_.erase(iter); } } @@ -788,12 +791,15 @@ void BaseWindow::RemoveBrowserView(gin::Handle browser_view) { void BaseWindow::SetTopBrowserView(gin::Handle browser_view, gin_helper::Arguments* args) { BaseWindow* owner_window = browser_view->owner_window(); - auto iter = browser_views_.find(browser_view->ID()); + auto iter = std::find(browser_views_.begin(), browser_views_.end(), + browser_view.ToV8()); if (iter == browser_views_.end() || (owner_window && owner_window != this)) { args->ThrowError("Given BrowserView is not attached to the window"); return; } + browser_views_.erase(iter); + browser_views_.emplace_back().Reset(isolate(), browser_view.ToV8()); window_->SetTopBrowserView(browser_view->view()); } @@ -1000,7 +1006,7 @@ v8::Local BaseWindow::GetBrowserView( return v8::Null(isolate()); } else if (browser_views_.size() == 1) { auto first_view = browser_views_.begin(); - return v8::Local::New(isolate(), (*first_view).second); + return v8::Local::New(isolate(), *first_view); } else { args->ThrowError( "BrowserWindow have multiple BrowserViews, " @@ -1012,8 +1018,8 @@ v8::Local BaseWindow::GetBrowserView( std::vector> BaseWindow::GetBrowserViews() const { std::vector> ret; - for (auto const& views_iter : browser_views_) { - ret.push_back(v8::Local::New(isolate(), views_iter.second)); + for (auto const& browser_view : browser_views_) { + ret.push_back(v8::Local::New(isolate(), browser_view)); } return ret; @@ -1122,7 +1128,7 @@ void BaseWindow::ResetBrowserViews() { for (auto& item : browser_views_) { gin::Handle browser_view; if (gin::ConvertFromV8(isolate(), - v8::Local::New(isolate(), item.second), + v8::Local::New(isolate(), item), &browser_view) && !browser_view.IsEmpty()) { // There's a chance that the BrowserView may have been reparented - only @@ -1135,7 +1141,7 @@ void BaseWindow::ResetBrowserViews() { browser_view->SetOwnerWindow(nullptr); } - item.second.Reset(); + item.Reset(); } browser_views_.clear(); diff --git a/shell/browser/api/electron_api_base_window.h b/shell/browser/api/electron_api_base_window.h index ba5c0399cf2a..3f2a02eb4057 100644 --- a/shell/browser/api/electron_api_base_window.h +++ b/shell/browser/api/electron_api_base_window.h @@ -275,7 +275,7 @@ class BaseWindow : public gin_helper::TrackableObject, #endif v8::Global content_view_; - std::map> browser_views_; + std::vector> browser_views_; v8::Global menu_; v8::Global parent_window_; KeyWeakMap child_windows_; diff --git a/spec/api-browser-view-spec.ts b/spec/api-browser-view-spec.ts index 3932510d2118..a45790644b53 100644 --- a/spec/api-browser-view-spec.ts +++ b/spec/api-browser-view-spec.ts @@ -286,6 +286,23 @@ describe('BrowserView module', () => { expect(views[0].webContents.id).to.equal(view1.webContents.id); expect(views[1].webContents.id).to.equal(view2.webContents.id); }); + + it('persists ordering by z-index', () => { + const view1 = new BrowserView(); + defer(() => view1.webContents.destroy()); + w.addBrowserView(view1); + defer(() => w.removeBrowserView(view1)); + const view2 = new BrowserView(); + defer(() => view2.webContents.destroy()); + w.addBrowserView(view2); + defer(() => w.removeBrowserView(view2)); + w.setTopBrowserView(view1); + + const views = w.getBrowserViews(); + expect(views).to.have.lengthOf(2); + expect(views[0].webContents.id).to.equal(view2.webContents.id); + expect(views[1].webContents.id).to.equal(view1.webContents.id); + }); }); describe('BrowserWindow.setTopBrowserView()', () => {