Skip to content

Commit

Permalink
fix: multiple dock icons when calling dock.show/hide (#25300)
Browse files Browse the repository at this point in the history
* fix: mulitple dock icons when calling dock.show/hide

* test: run dock.show tests after dock.hide tests

Co-authored-by: Cheng Zhao <zcbenz@gmail.com>
  • Loading branch information
trop[bot] and zcbenz committed Sep 3, 2020
1 parent 59f9e41 commit c3ad33c
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 7 deletions.
1 change: 1 addition & 0 deletions shell/browser/browser.h
Expand Up @@ -355,6 +355,7 @@ class Browser : public WindowListObserver {

#if defined(OS_MAC)
std::unique_ptr<ui::ScopedPasswordInputEnabler> password_input_enabler_;
base::Time last_dock_show_;
#endif

#if defined(OS_LINUX) || defined(OS_WIN)
Expand Down
15 changes: 15 additions & 0 deletions shell/browser/browser_mac.mm
Expand Up @@ -383,6 +383,20 @@ void RemoveFromLoginItems() {
}

void Browser::DockHide() {
// Transforming application state from UIElement to Foreground is an
// asyncronous operation, and unfortunately there is currently no way to know
// when it is finished.
// So if we call DockHide => DockShow => DockHide => DockShow in a very short
// time, we would triger a bug of macOS that, there would be multiple dock
// icons of the app left in system.
// To work around this, we make sure DockHide does nothing if it is called
// immediately after DockShow. After some experiments, 1 second seems to be
// a proper interval.
if (!last_dock_show_.is_null() &&
base::Time::Now() - last_dock_show_ < base::TimeDelta::FromSeconds(1)) {
return;
}

for (auto* const& window : WindowList::GetWindows())
[window->GetNativeWindow().GetNativeNSWindow() setCanHide:NO];

Expand All @@ -398,6 +412,7 @@ void RemoveFromLoginItems() {
}

v8::Local<v8::Promise> Browser::DockShow(v8::Isolate* isolate) {
last_dock_show_ = base::Time::Now();
gin_helper::Promise<void> promise(isolate);
v8::Local<v8::Promise> handle = promise.GetHandle();

Expand Down
17 changes: 10 additions & 7 deletions spec-main/api-app-spec.ts
Expand Up @@ -1437,6 +1437,16 @@ describe('app module', () => {
});
});

describe('dock.hide', () => {
it('should not throw', () => {
app.dock.hide();
expect(app.dock.isVisible()).to.equal(false);
});
});

// Note that dock.show tests should run after dock.hide tests, to work
// around a bug of macOS.
// See https://github.com/electron/electron/pull/25269 for more.
describe('dock.show', () => {
it('should not throw', () => {
return app.dock.show().then(() => {
Expand All @@ -1452,13 +1462,6 @@ describe('app module', () => {
await expect(app.dock.show()).to.eventually.be.fulfilled.equal(undefined);
});
});

describe('dock.hide', () => {
it('should not throw', () => {
app.dock.hide();
expect(app.dock.isVisible()).to.equal(false);
});
});
});

describe('whenReady', () => {
Expand Down

0 comments on commit c3ad33c

Please sign in to comment.