Skip to content

Commit

Permalink
Revert of ash: Use system tray mojo interface to show system update t…
Browse files Browse the repository at this point in the history
…ray icon (patchset chromium#6 id:100001 of https://codereview.chromium.org/2558043006/ )

Reason for revert:
Broke compile on Google Chrome ChromeOS:

https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20ChromeOS/builds/26896

../../chrome/browser/component_updater/pepper_flash_component_installer.cc:94:3: error: no type named 'SystemTrayClient' in namespace 'chromeos'; did you mean simply 'SystemTrayClient'?
  chromeos::SystemTrayClient* tray = chromeos::SystemTrayClient::Get();
  ^~~~~~~~~~~~~~~~~~~~~~~~~~
  SystemTrayClient
../../chrome/browser/ui/ash/system_tray_client.h:27:7: note: 'SystemTrayClient' declared here
class SystemTrayClient : public ash::mojom::SystemTrayClient,
      ^
../../chrome/browser/component_updater/pepper_flash_component_installer.cc:94:38: error: no member named 'SystemTrayClient' in namespace 'chromeos'; did you mean simply 'SystemTrayClient'?
  chromeos::SystemTrayClient* tray = chromeos::SystemTrayClient::Get();
                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~
                                     SystemTrayClient
../../chrome/browser/ui/ash/system_tray_client.h:27:7: note: 'SystemTrayClient' declared here
class SystemTrayClient : public ash::mojom::SystemTrayClient,
      ^
2 errors generated.

Original issue's description:
> ash: Use system tray mojo interface to show system update tray icon
>
> Change the flow so that Chrome explicitly asks ash to show the icon
> rather than having ash ask Chrome whether there is an update available.
>
> * Add ShowUpdateIcon to system_tray.mojom
> * Introduce update.mojom
> * Migrate ash to using ash::mojom::UpdateSeverity internally
> * Migrate update methods from SystemTrayDelegate to SystemTrayClient
> * Add a new SystemTrayClientTest and move existing Flash test there
>
> Also add docs for SystemTrayItem (used by TrayUpdate) since tray view
> vs. default view vs. detailed view always confuses me.
>
> BUG=647412
> TEST=chrome browser_tests, ash_unittests
>
> Committed: https://crrev.com/196ee3b66b3700733cecb8472d63d872d99e3783
> Cr-Commit-Position: refs/heads/master@{#438051}

TBR=msw@chromium.org,kerrnel@chromium.org,tsepez@chromium.org,waffles@chromium.org,sky@chromium.org,jamescook@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=647412

Review-Url: https://codereview.chromium.org/2568413002
Cr-Commit-Position: refs/heads/master@{#438064}
  • Loading branch information
yutak authored and Commit bot committed Dec 13, 2016
1 parent 82298e5 commit 9dac095
Show file tree
Hide file tree
Showing 34 changed files with 317 additions and 272 deletions.
1 change: 1 addition & 0 deletions ash/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,7 @@ component("ash") {
"common/system/tray_accessibility.h",
"common/system/update/tray_update.cc",
"common/system/update/tray_update.h",
"common/system/update/update_observer.h",
"common/system/user/button_from_view.cc",
"common/system/user/button_from_view.h",
"common/system/user/login_status.cc",
Expand Down
12 changes: 12 additions & 0 deletions ash/common/system/tray/default_system_tray_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@
#include "ash/common/system/tray/default_system_tray_delegate.h"

#include <string>
#include <utility>

#include "ash/common/system/networking_config_delegate.h"
#include "base/message_loop/message_loop.h"
#include "base/time/time.h"

namespace ash {

Expand All @@ -27,6 +32,13 @@ bool DefaultSystemTrayDelegate::IsUserSupervised() const {
return GetUserLoginStatus() == LoginStatus::SUPERVISED;
}

void DefaultSystemTrayDelegate::GetSystemUpdateInfo(UpdateInfo* info) const {
DCHECK(info);
info->severity = UpdateInfo::UPDATE_NONE;
info->update_required = true;
info->factory_reset_required = false;
}

bool DefaultSystemTrayDelegate::ShouldShowSettings() const {
return true;
}
Expand Down
1 change: 1 addition & 0 deletions ash/common/system/tray/default_system_tray_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class ASH_EXPORT DefaultSystemTrayDelegate : public SystemTrayDelegate {
LoginStatus GetUserLoginStatus() const override;
std::string GetSupervisedUserManager() const override;
bool IsUserSupervised() const override;
void GetSystemUpdateInfo(UpdateInfo* info) const override;
bool ShouldShowSettings() const override;
bool ShouldShowNotificationTray() const override;
void ToggleBluetooth() override;
Expand Down
4 changes: 4 additions & 0 deletions ash/common/system/tray/system_tray.cc
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,10 @@ TrayTiles* SystemTray::GetTrayTilesForTesting() const {
return tray_tiles_;
}

TrayUpdate* SystemTray::GetTrayUpdateForTesting() const {
return tray_update_;
}

void SystemTray::CloseBubble(const ui::KeyEvent& key_event) {
CloseSystemBubble();
}
Expand Down
3 changes: 1 addition & 2 deletions ash/common/system/tray/system_tray.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ class ASH_EXPORT SystemTray : public TrayBackgroundView,
explicit SystemTray(WmShelf* wm_shelf);
~SystemTray() override;

TrayUpdate* tray_update() { return tray_update_; }

// Calls TrayBackgroundView::Initialize(), creates the tray items, and
// adds them to SystemTrayNotifier.
void InitializeTrayItems(SystemTrayDelegate* delegate,
Expand Down Expand Up @@ -170,6 +168,7 @@ class ASH_EXPORT SystemTray : public TrayBackgroundView,
TrayNetwork* GetTrayNetworkForTesting() const;
TraySystemInfo* GetTraySystemInfoForTesting() const;
TrayTiles* GetTrayTilesForTesting() const;
TrayUpdate* GetTrayUpdateForTesting() const;

// Activates the system tray bubble.
void ActivateBubble();
Expand Down
16 changes: 0 additions & 16 deletions ash/common/system/tray/system_tray_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,8 @@

#include "ash/common/system/tray/system_tray_controller.h"

#include "ash/common/system/tray/system_tray.h"
#include "ash/common/system/tray/system_tray_notifier.h"
#include "ash/common/system/update/tray_update.h"
#include "ash/common/wm_root_window_controller.h"
#include "ash/common/wm_shell.h"
#include "ash/common/wm_window.h"

namespace ash {

Expand Down Expand Up @@ -132,16 +128,4 @@ void SystemTrayController::SetUse24HourClock(bool use_24_hour) {
WmShell::Get()->system_tray_notifier()->NotifyDateFormatChanged();
}

void SystemTrayController::ShowUpdateIcon(mojom::UpdateSeverity severity,
bool factory_reset_required) {
// Show the icon on all displays.
for (WmWindow* root : WmShell::Get()->GetAllRootWindows()) {
ash::SystemTray* tray = root->GetRootWindowController()->GetSystemTray();
// External monitors might not have a tray yet.
if (!tray)
continue;
tray->tray_update()->ShowUpdateIcon(severity, factory_reset_required);
}
}

} // namespace ash
6 changes: 2 additions & 4 deletions ash/common/system/tray/system_tray_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,11 @@ class ASH_EXPORT SystemTrayController
// Binds the mojom::SystemTray interface to this object.
void BindRequest(mojom::SystemTrayRequest request);

// mojom::SystemTray overrides. Public for testing.
private:
// mojom::SystemTray:
void SetClient(mojom::SystemTrayClientPtr client) override;
void SetUse24HourClock(bool use_24_hour) override;
void ShowUpdateIcon(mojom::UpdateSeverity severity,
bool factory_reset_required) override;

private:
// Client interface in chrome browser. Only bound on Chrome OS.
mojom::SystemTrayClientPtr system_tray_client_;

Expand Down
13 changes: 13 additions & 0 deletions ash/common/system/tray/system_tray_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ BluetoothDeviceInfo::BluetoothDeviceInfo(const BluetoothDeviceInfo& other) =

BluetoothDeviceInfo::~BluetoothDeviceInfo() {}

UpdateInfo::UpdateInfo()
: severity(UPDATE_NONE),
update_required(false),
factory_reset_required(false) {}

UpdateInfo::~UpdateInfo() {}

SystemTrayDelegate::SystemTrayDelegate() {}

SystemTrayDelegate::~SystemTrayDelegate() {}
Expand Down Expand Up @@ -59,6 +66,12 @@ bool SystemTrayDelegate::IsUserChild() const {
return false;
}

void SystemTrayDelegate::GetSystemUpdateInfo(UpdateInfo* info) const {
info->severity = UpdateInfo::UPDATE_NONE;
info->update_required = false;
info->factory_reset_required = false;
}

bool SystemTrayDelegate::ShouldShowSettings() const {
return false;
}
Expand Down
21 changes: 21 additions & 0 deletions ash/common/system/tray/system_tray_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,24 @@ struct ASH_EXPORT BluetoothDeviceInfo {

using BluetoothDeviceList = std::vector<BluetoothDeviceInfo>;

struct ASH_EXPORT UpdateInfo {
enum UpdateSeverity {
UPDATE_NONE,
UPDATE_LOW,
UPDATE_ELEVATED,
UPDATE_HIGH,
UPDATE_SEVERE,
UPDATE_CRITICAL,
};

UpdateInfo();
~UpdateInfo();

UpdateSeverity severity;
bool update_required;
bool factory_reset_required;
};

class NetworkingConfigDelegate;

// SystemTrayDelegate is intended for delegating tasks in the System Tray to the
Expand Down Expand Up @@ -105,6 +123,9 @@ class ASH_EXPORT SystemTrayDelegate {
// crbug.com/443119
virtual bool IsUserChild() const;

// Fills |info| structure (which must not be null) with current update info.
virtual void GetSystemUpdateInfo(UpdateInfo* info) const;

// Returns true if settings menu item should appear.
virtual bool ShouldShowSettings() const;

Expand Down
5 changes: 0 additions & 5 deletions ash/common/system/tray/system_tray_item.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,6 @@ class SystemTray;
class SystemTrayBubble;
class TrayItemView;

// Controller for an item in the system tray. Each item can create these views:
// Tray view - The icon in the status area in the shelf.
// Default view - The row in the top-level menu.
// Detailed view - The submenu shown when the top-level menu row is clicked.
class ASH_EXPORT SystemTrayItem {
public:
// The different types of SystemTrayItems.
Expand Down Expand Up @@ -91,7 +87,6 @@ class ASH_EXPORT SystemTrayItem {

// Returns a notification view for the item. This view is displayed with
// other notifications and should be the same size as default views.
// DEPRECATED. Use the message center instead.
virtual views::View* CreateNotificationView(LoginStatus status);

// These functions are called when the corresponding view item is about to be
Expand Down
14 changes: 14 additions & 0 deletions ash/common/system/tray/system_tray_notifier.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "ash/common/system/accessibility_observer.h"
#include "ash/common/system/date/clock_observer.h"
#include "ash/common/system/ime/ime_observer.h"
#include "ash/common/system/update/update_observer.h"
#include "ash/common/system/user/user_observer.h"

#if defined(OS_CHROMEOS)
Expand Down Expand Up @@ -91,6 +92,19 @@ void SystemTrayNotifier::NotifyRefreshIMEMenu(bool is_active) {
observer.OnIMEMenuActivationChanged(is_active);
}

void SystemTrayNotifier::AddUpdateObserver(UpdateObserver* observer) {
update_observers_.AddObserver(observer);
}

void SystemTrayNotifier::RemoveUpdateObserver(UpdateObserver* observer) {
update_observers_.RemoveObserver(observer);
}

void SystemTrayNotifier::NotifyUpdateRecommended(const UpdateInfo& info) {
for (auto& observer : update_observers_)
observer.OnUpdateRecommended(info);
}

void SystemTrayNotifier::AddUserObserver(UserObserver* observer) {
user_observers_.AddObserver(observer);
}
Expand Down
12 changes: 8 additions & 4 deletions ash/common/system/tray/system_tray_notifier.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ namespace ash {
class AccessibilityObserver;
class ClockObserver;
class IMEObserver;
struct UpdateInfo;
class UpdateObserver;
class UserObserver;

#if defined(OS_CHROMEOS)
Expand All @@ -38,10 +40,6 @@ class TracingObserver;
class VirtualKeyboardObserver;
#endif

namespace mojom {
enum class UpdateSeverity;
}

// Observer and notification manager for the ash system tray.
class ASH_EXPORT SystemTrayNotifier {
public:
Expand All @@ -68,6 +66,11 @@ class ASH_EXPORT SystemTrayNotifier {
void NotifyRefreshIME();
void NotifyRefreshIMEMenu(bool is_active);

// OS updates.
void AddUpdateObserver(UpdateObserver* observer);
void RemoveUpdateObserver(UpdateObserver* observer);
void NotifyUpdateRecommended(const UpdateInfo& info);

// User.
void AddUserObserver(UserObserver* observer);
void RemoveUserObserver(UserObserver* observer);
Expand Down Expand Up @@ -144,6 +147,7 @@ class ASH_EXPORT SystemTrayNotifier {
base::ObserverList<AccessibilityObserver> accessibility_observers_;
base::ObserverList<ClockObserver> clock_observers_;
base::ObserverList<IMEObserver> ime_observers_;
base::ObserverList<UpdateObserver> update_observers_;
base::ObserverList<UserObserver> user_observers_;

#if defined(OS_CHROMEOS)
Expand Down
2 changes: 0 additions & 2 deletions ash/common/system/tray/tray_image_item.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ class ImageView;
namespace ash {
class TrayItemView;

// A system tray item that uses an image as its "tray view" in the status area.
class ASH_EXPORT TrayImageItem : public SystemTrayItem {
public:
TrayImageItem(SystemTray* system_tray, int resource_id, UmaType uma_type);
Expand Down Expand Up @@ -58,7 +57,6 @@ class ASH_EXPORT TrayImageItem : public SystemTrayItem {
// The color of the material design icon in the tray.
SkColor icon_color_;

// The image view in the tray.
TrayItemView* tray_view_;

DISALLOW_COPY_AND_ASSIGN(TrayImageItem);
Expand Down
Loading

0 comments on commit 9dac095

Please sign in to comment.