From 89de5b6e9a0170f3162da720fc3d4ca76dedb490 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 28 Nov 2014 17:47:47 +0800 Subject: [PATCH] win: Remove NotifyIconHostStateChanger We don't need the ability it provided, and it is causing crashes on some machines. Fixes #850. --- atom.gyp | 2 - atom/browser/ui/win/notify_icon.cc | 2 - atom/browser/ui/win/notify_icon_host.cc | 72 ------- atom/browser/ui/win/notify_icon_host.h | 16 -- .../status_icons/status_tray_state_changer_win.cc | 236 --------------------- .../status_icons/status_tray_state_changer_win.h | 133 ------------ 6 files changed, 461 deletions(-) delete mode 100644 chromium_src/chrome/browser/ui/views/status_icons/status_tray_state_changer_win.cc delete mode 100644 chromium_src/chrome/browser/ui/views/status_icons/status_tray_state_changer_win.h diff --git a/atom.gyp b/atom.gyp index a78ad70..e3c9eea 100644 --- a/atom.gyp +++ b/atom.gyp @@ -315,8 +315,6 @@ 'chromium_src/chrome/browser/ui/libgtk2ui/gtk2_status_icon.h', 'chromium_src/chrome/browser/ui/views/frame/global_menu_bar_registrar_x11.cc', 'chromium_src/chrome/browser/ui/views/frame/global_menu_bar_registrar_x11.h', - 'chromium_src/chrome/browser/ui/views/status_icons/status_tray_state_changer_win.cc', - 'chromium_src/chrome/browser/ui/views/status_icons/status_tray_state_changer_win.h', 'chromium_src/chrome/common/print_messages.cc', 'chromium_src/chrome/common/print_messages.h', 'chromium_src/chrome/common/tts_messages.h', diff --git a/atom/browser/ui/win/notify_icon.cc b/atom/browser/ui/win/notify_icon.cc index 5e004d8..7be52db 100644 --- a/atom/browser/ui/win/notify_icon.cc +++ b/atom/browser/ui/win/notify_icon.cc @@ -100,8 +100,6 @@ void NotifyIcon::SetImage(const gfx::ImageSkia& image) { BOOL result = Shell_NotifyIcon(NIM_MODIFY, &icon_data); if (!result) LOG(WARNING) << "Error setting status tray icon image"; - else - host_->UpdateIconVisibilityInBackground(this); } void NotifyIcon::SetPressedImage(const gfx::ImageSkia& image) { diff --git a/atom/browser/ui/win/notify_icon_host.cc b/atom/browser/ui/win/notify_icon_host.cc index db4f4d3..c9ef82a 100644 --- a/atom/browser/ui/win/notify_icon_host.cc +++ b/atom/browser/ui/win/notify_icon_host.cc @@ -12,7 +12,6 @@ #include "base/threading/non_thread_safe.h" #include "base/threading/thread.h" #include "base/win/wrapped_window_proc.h" -#include "chrome/browser/ui/views/status_icons/status_tray_state_changer_win.h" #include "ui/gfx/screen.h" #include "ui/gfx/win/hwnd_util.h" @@ -29,68 +28,6 @@ const wchar_t kNotifyIconHostWindowClass[] = L"AtomShell_NotifyIconHostWindow"; } // namespace -// Default implementation for NotifyIconHostStateChangerProxy that communicates -// to Exporer.exe via COM. It spawns a background thread with a fresh COM -// apartment and requests that the visibility be increased unless the user -// has explicitly set the icon to be hidden. -class NotifyIconHostStateChangerProxyImpl - : public NotifyIconHostStateChangerProxy, - public base::NonThreadSafe { - public: - NotifyIconHostStateChangerProxyImpl() - : pending_requests_(0), - worker_thread_("NotifyIconCOMWorkerThread"), - weak_factory_(this) { - worker_thread_.init_com_with_mta(false); - } - - virtual void EnqueueChange(UINT icon_id, HWND window) OVERRIDE { - DCHECK(CalledOnValidThread()); - if (pending_requests_ == 0) - worker_thread_.Start(); - - ++pending_requests_; - worker_thread_.message_loop_proxy()->PostTaskAndReply( - FROM_HERE, - base::Bind( - &NotifyIconHostStateChangerProxyImpl::EnqueueChangeOnWorkerThread, - icon_id, - window), - base::Bind(&NotifyIconHostStateChangerProxyImpl::ChangeDone, - weak_factory_.GetWeakPtr())); - } - - private: - // Must be called only on |worker_thread_|, to ensure the correct COM - // apartment. - static void EnqueueChangeOnWorkerThread(UINT icon_id, HWND window) { - // It appears that IUnknowns are coincidentally compatible with - // scoped_refptr. Normally I wouldn't depend on that but it seems that - // base::win::IUnknownImpl itself depends on that coincidence so it's - // already being assumed elsewhere. - scoped_refptr status_tray_state_changer( - new StatusTrayStateChangerWin(icon_id, window)); - status_tray_state_changer->EnsureTrayIconVisible(); - } - - // Called on UI thread. - void ChangeDone() { - DCHECK(CalledOnValidThread()); - DCHECK_GT(pending_requests_, 0); - - if (--pending_requests_ == 0) - worker_thread_.Stop(); - } - - private: - int pending_requests_; - base::Thread worker_thread_; - base::WeakPtrFactory weak_factory_; - - DISALLOW_COPY_AND_ASSIGN(NotifyIconHostStateChangerProxyImpl); -}; - - NotifyIconHost::NotifyIconHost() : next_icon_id_(1), atom_(0), @@ -151,15 +88,6 @@ void NotifyIconHost::Remove(NotifyIcon* icon) { notify_icons_.erase(i); } -void NotifyIconHost::UpdateIconVisibilityInBackground( - NotifyIcon* notify_icon) { - if (!state_changer_proxy_.get()) - state_changer_proxy_.reset(new NotifyIconHostStateChangerProxyImpl); - - state_changer_proxy_->EnqueueChange(notify_icon->icon_id(), - notify_icon->window()); -} - LRESULT CALLBACK NotifyIconHost::WndProcStatic(HWND hwnd, UINT message, WPARAM wparam, diff --git a/atom/browser/ui/win/notify_icon_host.h b/atom/browser/ui/win/notify_icon_host.h index 0481d89..6797d4f 100644 --- a/atom/browser/ui/win/notify_icon_host.h +++ b/atom/browser/ui/win/notify_icon_host.h @@ -16,16 +16,6 @@ namespace atom { class NotifyIcon; -// A class that's responsible for increasing, if possible, the visibility -// of a status tray icon on the taskbar. The default implementation sends -// a task to a worker thread each time EnqueueChange is called. -class NotifyIconHostStateChangerProxy { - public: - // Called by NotifyIconHost to request upgraded visibility on the icon - // represented by the |icon_id|, |window| pair. - virtual void EnqueueChange(UINT icon_id, HWND window) = 0; -}; - class NotifyIconHost { public: NotifyIconHost(); @@ -34,8 +24,6 @@ class NotifyIconHost { NotifyIcon* CreateNotifyIcon(); void Remove(NotifyIcon* notify_icon); - void UpdateIconVisibilityInBackground(NotifyIcon* notify_icon); - private: typedef std::vector NotifyIcons; @@ -67,10 +55,6 @@ class NotifyIconHost { // reset our status icons. UINT taskbar_created_message_; - // Manages changes performed on a background thread to manipulate visibility - // of notification icons. - scoped_ptr state_changer_proxy_; - DISALLOW_COPY_AND_ASSIGN(NotifyIconHost); }; diff --git a/chromium_src/chrome/browser/ui/views/status_icons/status_tray_state_changer_win.cc b/chromium_src/chrome/browser/ui/views/status_icons/status_tray_state_changer_win.cc deleted file mode 100644 index 410d31d..0000000 --- a/chromium_src/chrome/browser/ui/views/status_icons/status_tray_state_changer_win.cc +++ /dev/null @@ -1,236 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "chrome/browser/ui/views/status_icons/status_tray_state_changer_win.h" - -namespace { - -//////////////////////////////////////////////////////////////////////////////// -// Status Tray API - -// The folowing describes the interface to the undocumented Windows Exporer APIs -// for manipulating with the status tray area. This code should be used with -// care as it can change with versions (even minor versions) of Windows. - -// ITrayNotify is an interface describing the API for manipulating the state of -// the Windows notification area, as well as for registering for change -// notifications. -class __declspec(uuid("FB852B2C-6BAD-4605-9551-F15F87830935")) ITrayNotify - : public IUnknown { - public: - virtual HRESULT STDMETHODCALLTYPE - RegisterCallback(INotificationCB* callback) = 0; - virtual HRESULT STDMETHODCALLTYPE - SetPreference(const NOTIFYITEM* notify_item) = 0; - virtual HRESULT STDMETHODCALLTYPE EnableAutoTray(BOOL enabled) = 0; -}; - -// ITrayNotifyWin8 is the interface that replaces ITrayNotify for newer versions -// of Windows. -class __declspec(uuid("D133CE13-3537-48BA-93A7-AFCD5D2053B4")) ITrayNotifyWin8 - : public IUnknown { - public: - virtual HRESULT STDMETHODCALLTYPE - RegisterCallback(INotificationCB* callback, unsigned long*) = 0; - virtual HRESULT STDMETHODCALLTYPE UnregisterCallback(unsigned long*) = 0; - virtual HRESULT STDMETHODCALLTYPE SetPreference(NOTIFYITEM const*) = 0; - virtual HRESULT STDMETHODCALLTYPE EnableAutoTray(BOOL) = 0; - virtual HRESULT STDMETHODCALLTYPE DoAction(BOOL) = 0; -}; - -const CLSID CLSID_TrayNotify = { - 0x25DEAD04, - 0x1EAC, - 0x4911, - {0x9E, 0x3A, 0xAD, 0x0A, 0x4A, 0xB5, 0x60, 0xFD}}; - -} // namespace - -StatusTrayStateChangerWin::StatusTrayStateChangerWin(UINT icon_id, HWND window) - : interface_version_(INTERFACE_VERSION_UNKNOWN), - icon_id_(icon_id), - window_(window) { - wchar_t module_name[MAX_PATH]; - ::GetModuleFileName(NULL, module_name, MAX_PATH); - - file_name_ = module_name; -} - -void StatusTrayStateChangerWin::EnsureTrayIconVisible() { - DCHECK(CalledOnValidThread()); - - if (!CreateTrayNotify()) { - VLOG(1) << "Unable to create COM object for ITrayNotify."; - return; - } - - scoped_ptr notify_item = RegisterCallback(); - - // If the user has already hidden us explicitly, try to honor their choice by - // not changing anything. - if (notify_item->preference == PREFERENCE_SHOW_NEVER) - return; - - // If we are already on the taskbar, return since nothing needs to be done. - if (notify_item->preference == PREFERENCE_SHOW_ALWAYS) - return; - - notify_item->preference = PREFERENCE_SHOW_ALWAYS; - - SendNotifyItemUpdate(notify_item.Pass()); -} - -STDMETHODIMP_(ULONG) StatusTrayStateChangerWin::AddRef() { - DCHECK(CalledOnValidThread()); - return base::win::IUnknownImpl::AddRef(); -} - -STDMETHODIMP_(ULONG) StatusTrayStateChangerWin::Release() { - DCHECK(CalledOnValidThread()); - return base::win::IUnknownImpl::Release(); -} - -STDMETHODIMP StatusTrayStateChangerWin::QueryInterface(REFIID riid, - PVOID* ptr_void) { - DCHECK(CalledOnValidThread()); - if (riid == __uuidof(INotificationCB)) { - *ptr_void = static_cast(this); - AddRef(); - return S_OK; - } - - return base::win::IUnknownImpl::QueryInterface(riid, ptr_void); -} - -STDMETHODIMP StatusTrayStateChangerWin::Notify(ULONG event, - NOTIFYITEM* notify_item) { - DCHECK(CalledOnValidThread()); - DCHECK(notify_item); - if (notify_item->hwnd != window_ || notify_item->id != icon_id_ || - base::string16(notify_item->exe_name) != file_name_) { - return S_OK; - } - - notify_item_.reset(new NOTIFYITEM(*notify_item)); - return S_OK; -} - -StatusTrayStateChangerWin::~StatusTrayStateChangerWin() { - DCHECK(CalledOnValidThread()); -} - -bool StatusTrayStateChangerWin::CreateTrayNotify() { - DCHECK(CalledOnValidThread()); - - tray_notify_.Release(); // Release so this method can be called more than - // once. - - HRESULT hr = tray_notify_.CreateInstance(CLSID_TrayNotify); - if (FAILED(hr)) - return false; - - base::win::ScopedComPtr tray_notify_win8; - hr = tray_notify_win8.QueryFrom(tray_notify_); - if (SUCCEEDED(hr)) { - interface_version_ = INTERFACE_VERSION_WIN8; - return true; - } - - base::win::ScopedComPtr tray_notify_legacy; - hr = tray_notify_legacy.QueryFrom(tray_notify_); - if (SUCCEEDED(hr)) { - interface_version_ = INTERFACE_VERSION_LEGACY; - return true; - } - - return false; -} - -scoped_ptr StatusTrayStateChangerWin::RegisterCallback() { - // |notify_item_| is used to store the result of the callback from - // Explorer.exe, which happens synchronously during - // RegisterCallbackWin8 or RegisterCallbackLegacy. - DCHECK(notify_item_.get() == NULL); - - // TODO(dewittj): Add UMA logging here to report if either of our strategies - // has a tendency to fail on particular versions of Windows. - switch (interface_version_) { - case INTERFACE_VERSION_WIN8: - if (!RegisterCallbackWin8()) - VLOG(1) << "Unable to successfully run RegisterCallbackWin8."; - break; - case INTERFACE_VERSION_LEGACY: - if (!RegisterCallbackLegacy()) - VLOG(1) << "Unable to successfully run RegisterCallbackLegacy."; - break; - default: - NOTREACHED(); - } - - // Adding an intermediate scoped pointer here so that |notify_item_| is reset - // to NULL. - scoped_ptr rv(notify_item_.release()); - return rv.Pass(); -} - -bool StatusTrayStateChangerWin::RegisterCallbackWin8() { - base::win::ScopedComPtr tray_notify_win8; - HRESULT hr = tray_notify_win8.QueryFrom(tray_notify_); - if (FAILED(hr)) - return false; - - // The following two lines cause Windows Explorer to call us back with all the - // existing tray icons and their preference. It would also presumably notify - // us if changes were made in realtime while we registered as a callback, but - // we just want to modify our own entry so we immediately unregister. - unsigned long callback_id = 0; - hr = tray_notify_win8->RegisterCallback(this, &callback_id); - tray_notify_win8->UnregisterCallback(&callback_id); - if (FAILED(hr)) { - return false; - } - - return true; -} - -bool StatusTrayStateChangerWin::RegisterCallbackLegacy() { - base::win::ScopedComPtr tray_notify; - HRESULT hr = tray_notify.QueryFrom(tray_notify_); - if (FAILED(hr)) { - return false; - } - - // The following two lines cause Windows Explorer to call us back with all the - // existing tray icons and their preference. It would also presumably notify - // us if changes were made in realtime while we registered as a callback. In - // this version of the API, there can be only one registered callback so it is - // better to unregister as soon as possible. - // TODO(dewittj): Try to notice if the notification area icon customization - // window is open and postpone this call until the user closes it; - // registering the callback while the window is open can cause stale data to - // be displayed to the user. - hr = tray_notify->RegisterCallback(this); - tray_notify->RegisterCallback(NULL); - if (FAILED(hr)) { - return false; - } - - return true; -} - -void StatusTrayStateChangerWin::SendNotifyItemUpdate( - scoped_ptr notify_item) { - if (interface_version_ == INTERFACE_VERSION_LEGACY) { - base::win::ScopedComPtr tray_notify; - HRESULT hr = tray_notify.QueryFrom(tray_notify_); - if (SUCCEEDED(hr)) - tray_notify->SetPreference(notify_item.get()); - } else if (interface_version_ == INTERFACE_VERSION_WIN8) { - base::win::ScopedComPtr tray_notify; - HRESULT hr = tray_notify.QueryFrom(tray_notify_); - if (SUCCEEDED(hr)) - tray_notify->SetPreference(notify_item.get()); - } -} - diff --git a/chromium_src/chrome/browser/ui/views/status_icons/status_tray_state_changer_win.h b/chromium_src/chrome/browser/ui/views/status_icons/status_tray_state_changer_win.h deleted file mode 100644 index 963792b..0000000 --- a/chromium_src/chrome/browser/ui/views/status_icons/status_tray_state_changer_win.h +++ /dev/null @@ -1,133 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef CHROME_BROWSER_UI_VIEWS_STATUS_ICONS_STATUS_TRAY_STATE_CHANGER_WIN_H_ -#define CHROME_BROWSER_UI_VIEWS_STATUS_ICONS_STATUS_TRAY_STATE_CHANGER_WIN_H_ - -#include "base/memory/scoped_ptr.h" -#include "base/strings/string16.h" -#include "base/threading/non_thread_safe.h" -#include "base/win/iunknown_impl.h" -#include "base/win/scoped_comptr.h" - -// The known values for NOTIFYITEM's dwPreference member. -enum NOTIFYITEM_PREFERENCE { - // In Windows UI: "Only show notifications." - PREFERENCE_SHOW_WHEN_ACTIVE = 0, - // In Windows UI: "Hide icon and notifications." - PREFERENCE_SHOW_NEVER = 1, - // In Windows UI: "Show icon and notifications." - PREFERENCE_SHOW_ALWAYS = 2 -}; - -// NOTIFYITEM describes an entry in Explorer's registry of status icons. -// Explorer keeps entries around for a process even after it exits. -struct NOTIFYITEM { - PWSTR exe_name; // The file name of the creating executable. - PWSTR tip; // The last hover-text value associated with this status - // item. - HICON icon; // The icon associated with this status item. - HWND hwnd; // The HWND associated with the status item. - DWORD preference; // Determines the behavior of the icon with respect to - // the taskbar. Values taken from NOTIFYITEM_PREFERENCE. - UINT id; // The ID specified by the application. (hWnd, uID) is - // unique. - GUID guid; // The GUID specified by the application, alternative to - // uID. -}; - -// INotificationCB is an interface that applications can implement in order to -// receive notifications about the state of the notification area manager. -class __declspec(uuid("D782CCBA-AFB0-43F1-94DB-FDA3779EACCB")) INotificationCB - : public IUnknown { - public: - virtual HRESULT STDMETHODCALLTYPE - Notify(ULONG event, NOTIFYITEM* notify_item) = 0; -}; - -// A class that is capable of reading and writing the state of the notification -// area in the Windows taskbar. It is used to promote a tray icon from the -// overflow area to the taskbar, and refuses to do anything if the user has -// explicitly marked an icon to be always hidden. -class StatusTrayStateChangerWin : public INotificationCB, - public base::win::IUnknownImpl, - public base::NonThreadSafe { - public: - StatusTrayStateChangerWin(UINT icon_id, HWND window); - - // Call this method to move the icon matching |icon_id| and |window| to the - // taskbar from the overflow area. This will not make any changes if the - // icon has been set to |PREFERENCE_SHOW_NEVER|, in order to comply with - // the explicit wishes/configuration of the user. - void EnsureTrayIconVisible(); - - // IUnknown. - virtual ULONG STDMETHODCALLTYPE AddRef() OVERRIDE; - virtual ULONG STDMETHODCALLTYPE Release() OVERRIDE; - virtual HRESULT STDMETHODCALLTYPE QueryInterface(REFIID, PVOID*) OVERRIDE; - - // INotificationCB. - // Notify is called in response to RegisterCallback for each current - // entry in Explorer's list of notification area icons, and ever time - // one of them changes, until UnregisterCallback is called or |this| - // is destroyed. - virtual HRESULT STDMETHODCALLTYPE Notify(ULONG, NOTIFYITEM*); - - protected: - virtual ~StatusTrayStateChangerWin(); - - private: - friend class StatusTrayStateChangerWinTest; - - enum InterfaceVersion { - INTERFACE_VERSION_LEGACY = 0, - INTERFACE_VERSION_WIN8, - INTERFACE_VERSION_UNKNOWN - }; - - // Creates an instance of TrayNotify, and ensures that it supports either - // ITrayNotify or ITrayNotifyWin8. Returns true on success. - bool CreateTrayNotify(); - - // Returns the NOTIFYITEM that corresponds to this executable and the - // HWND/ID pair that were used to create the StatusTrayStateChangerWin. - // Internally it calls the appropriate RegisterCallback{Win8,Legacy}. - scoped_ptr RegisterCallback(); - - // Calls RegisterCallback with the appropriate interface required by - // different versions of Windows. This will result in |notify_item_| being - // updated when a matching item is passed into - // StatusTrayStateChangerWin::Notify. - bool RegisterCallbackWin8(); - bool RegisterCallbackLegacy(); - - // Sends an update to Explorer with the passed NOTIFYITEM. - void SendNotifyItemUpdate(scoped_ptr notify_item); - - // Storing IUnknown since we will need to use different interfaces - // for different versions of Windows. - base::win::ScopedComPtr tray_notify_; - InterfaceVersion interface_version_; - - // The ID assigned to the notification area icon that we want to manipulate. - const UINT icon_id_; - // The HWND associated with the notification area icon that we want to - // manipulate. This is an unretained pointer, do not dereference. - const HWND window_; - // Executable name of the current program. Along with |icon_id_| and - // |window_|, this uniquely identifies a notification area entry to Explorer. - base::string16 file_name_; - - // Temporary storage for the matched NOTIFYITEM. This is necessary because - // Notify doesn't return anything. The call flow looks like this: - // TrayNotify->RegisterCallback() - // ... other COM stack frames .. - // StatusTrayStateChangerWin->Notify(NOTIFYITEM); - // so we can't just return the notifyitem we're looking for. - scoped_ptr notify_item_; - - DISALLOW_COPY_AND_ASSIGN(StatusTrayStateChangerWin); -}; - -#endif // CHROME_BROWSER_UI_VIEWS_STATUS_ICONS_STATUS_TRAY_STATE_CHANGER_WIN_H_ \ No newline at end of file -- 2.7.4