mirrors/electron
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions

fix: WebContentsView removal should compare directly (#44673)

Browse source 
fix: `WebContentsView` removal should compare directly (#44656)

* fix: WebContentsView removal should compare directly

* fixup view comparision

* chore: use erase_if

* Apply review suggestions

---------

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
This commit is contained in:
John Kleinschmidt 2024-11-15 07:09:47 -05:00 committed by GitHub
parent 9c5c5d0fea
commit 2bd3a9fe65
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 43 additions and 26 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

37
shell/browser/api/electron_api_view.cc
View file

@ -122,15 +122,6 @@ struct Converter<views::FlexAllocationOrder> {
}
};
template <>
struct Converter<electron::api::View> {
static bool FromV8(v8::Isolate* isolate,
v8::Local<v8::Value> val,
electron::api::View* out) {
return gin::ConvertFromV8(isolate, val, &out);
}
};
template <>
struct Converter<views::SizeBound> {
static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
@ -193,7 +184,10 @@ View::~View() {
void View::ReorderChildView(gin::Handle<View> child, size_t index) {
view_->ReorderChildView(child->view(), index);
const auto i = base::ranges::find(child_views_, child.ToV8());
const auto i =
std::ranges::find_if(child_views_, [&](const ChildPair& child_view) {
return child_view.first == child->view();
});
DCHECK(i != child_views_.end());
// If |view| is already at the desired position, there's nothing to do.
@ -238,7 +232,8 @@ void View::AddChildViewAt(gin::Handle<View> child,
}
child_views_.emplace(child_views_.begin() + index, // index
isolate(), child->GetWrapper()); // v8::Global(args...)
child->view(),
v8::Global<v8::Object>(isolate(), child->GetWrapper()));
#if BUILDFLAG(IS_MAC)
// Disable the implicit CALayer animations that happen by default when adding
// or removing sublayers.
@ -258,7 +253,10 @@ void View::RemoveChildView(gin::Handle<View> child) {
if (!view_)
return;
const auto it = base::ranges::find(child_views_, child.ToV8());
const auto it =
std::ranges::find_if(child_views_, [&](const ChildPair& child_view) {
return child_view.first == child->view();
});
if (it != child_views_.end()) {
#if BUILDFLAG(IS_MAC)
ScopedCAActionDisabler disable_animations;
@ -336,8 +334,8 @@ std::vector<v8::Local<v8::Value>> View::GetChildren() {
v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
for (auto& child_view : child_views_)
ret.push_back(child_view.Get(isolate));
for (auto& [view, global] : child_views_)
ret.push_back(global.Get(isolate));
return ret;
}
@ -364,16 +362,9 @@ void View::OnViewIsDeleting(views::View* observed_view) {
}
void View::OnChildViewRemoved(views::View* observed_view, views::View* child) {
v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
auto it = std::ranges::find_if(
child_views_, [&](const v8::Global<v8::Object>& child_view) {
View current_view;
gin::ConvertFromV8(isolate, child_view.Get(isolate), &current_view);
return current_view.view()->GetID() == child->GetID();
std::erase_if(child_views_, [child](const ChildPair& child_view) {
return child_view.first == child;
});
if (it != child_views_.end()) {
child_views_.erase(it);
}
}
// static

4
shell/browser/api/electron_api_view.h
View file

@ -17,6 +17,8 @@
namespace electron::api {
using ChildPair = std::pair<raw_ptr<views::View>, v8::Global<v8::Object>>;
class View : public gin_helper::EventEmitter<View>, public views::ViewObserver {
public:
static gin_helper::WrappableBase* New(gin::Arguments* args);
@ -61,7 +63,7 @@ class View : public gin_helper::EventEmitter<View>, public views::ViewObserver {
private:
void ReorderChildView(gin::Handle<View> child, size_t index);
std::vector<v8::Global<v8::Object>> child_views_;
std::vector<ChildPair> child_views_;
bool delete_view_ = true;
raw_ptr<views::View> view_ = nullptr;

24
spec/api-web-contents-view-spec.ts
View file

@ -100,6 +100,30 @@ describe('WebContentsView', () => {
expect(w.contentView.children).to.deep.equal([wcv3, wcv1, wcv2]);
});
it('handle removal and re-addition of children', () => {
const w = new BaseWindow({ show: false });
const cv = new View();
w.setContentView(cv);
const wcv1 = new WebContentsView();
const wcv2 = new WebContentsView();
expect(w.contentView.children).to.deep.equal([]);
w.contentView.addChildView(wcv1);
w.contentView.addChildView(wcv2);
expect(w.contentView.children).to.deep.equal([wcv1, wcv2]);
w.contentView.removeChildView(wcv1);
expect(w.contentView.children).to.deep.equal([wcv2]);
w.contentView.addChildView(wcv1);
expect(w.contentView.children).to.deep.equal([wcv2, wcv1]);
});
function triggerGCByAllocation () {
const arr = [];
for (let i = 0; i < 1000000; i++) {