mash: Break ash system tray dependencies on ash::ShelfWidget

mash: Break ash system tray dependencies on ash::ShelfWidget

This is a step towards converting the system tray to wm common types, as well
as a step toward hiding the internal details of the shelf implementation from
external users. Long term, all access to shelf state should go via WmShelf.

* Plumb WmShelf into StatusAreaWidget.
* Remove ShelfWidget usage from //ash/system.
* Remove unnecessary includes.

BUG=615502, 619636
TEST=ash_unittests

Committed: https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee
Cr-Commit-Position: refs/heads/master@{#400520}

[James Cook, 2016-06-17 18:34:07]

jamescook@chromium.org changed reviewers:
+ msw@chromium.org

[James Cook, 2016-06-17 18:34:08]

msw, please take a look.

[msw, 2016-06-17 19:58:37]

lgtm with tangential minor comments.

https://codereview.chromium.org/2072023003/diff/1/ash/system/DEPS
File ash/system/DEPS (right):

https://codereview.chromium.org/2072023003/diff/1/ash/system/DEPS#newcode2
ash/system/DEPS:2: # jamescook@ is refactoring the system tray code to break
dependencies on the
optional nit: consider something like this one-liner:
  # NOTE: Use WmShelf for all external shelf access; see crbug.com/615502

https://codereview.chromium.org/2072023003/diff/1/ash/system/tray/system_tray...
File ash/system/tray/system_tray_unittest.cc (right):

https://codereview.chromium.org/2072023003/diff/1/ash/system/tray/system_tray...
ash/system/tray/system_tray_unittest.cc:352: StatusAreaWidget* widget =
StatusAreaWidgetTestHelper::GetStatusAreaWidget();
aside: This test is confusing... partly because of my weak terminology expertise
between the SystemTray and StatusAreaWidget... but maybe refactoring this with
Rect::Contains or EXPECT_GE, etc. would be nice (but not important here/now)

https://codereview.chromium.org/2072023003/diff/1/ash/system/tray/tray_backgr...
File ash/system/tray/tray_background_view.cc (right):

https://codereview.chromium.org/2072023003/diff/1/ash/system/tray/tray_backgr...
ash/system/tray/tray_background_view.cc:108: DCHECK(shelf);
q: why DCHECK here? nit: just one-line return tray_background_view_->GetShelf();

https://codereview.chromium.org/2072023003/diff/1/ash/system/tray/tray_detail...
File ash/system/tray/tray_details_view_unittest.cc (right):

https://codereview.chromium.org/2072023003/diff/1/ash/system/tray/tray_detail...
ash/system/tray/tray_details_view_unittest.cc:197: ui::test::EventGenerator&
generator = GetEventGenerator();
Hmm, I thought we discouraged non-const refs, but I guess that's mainly for
input parameters. Still, it'd be nice if this function returned a pointer.

[James Cook, 2016-06-17 20:14:32]

Thanks for the quick review.

https://codereview.chromium.org/2072023003/diff/1/ash/system/DEPS
File ash/system/DEPS (right):

https://codereview.chromium.org/2072023003/diff/1/ash/system/DEPS#newcode2
ash/system/DEPS:2: # jamescook@ is refactoring the system tray code to break
dependencies on the
On 2016/06/17 19:58:37, msw wrote:
> optional nit: consider something like this one-liner:
>   # NOTE: Use WmShelf for all external shelf access; see crbug.com/615502

Done.

https://codereview.chromium.org/2072023003/diff/1/ash/system/tray/system_tray...
File ash/system/tray/system_tray_unittest.cc (right):

https://codereview.chromium.org/2072023003/diff/1/ash/system/tray/system_tray...
ash/system/tray/system_tray_unittest.cc:352: StatusAreaWidget* widget =
StatusAreaWidgetTestHelper::GetStatusAreaWidget();
On 2016/06/17 19:58:37, msw wrote:
> aside: This test is confusing... partly because of my weak terminology
expertise
> between the SystemTray and StatusAreaWidget... but maybe refactoring this with
> Rect::Contains or EXPECT_GE, etc. would be nice (but not important here/now)

Converted to Rect::Contains() and clarified the test comment slightly.

https://codereview.chromium.org/2072023003/diff/1/ash/system/tray/tray_backgr...
File ash/system/tray/tray_background_view.cc (right):

https://codereview.chromium.org/2072023003/diff/1/ash/system/tray/tray_backgr...
ash/system/tray/tray_background_view.cc:108: DCHECK(shelf);
On 2016/06/17 19:58:37, msw wrote:
> q: why DCHECK here? nit: just one-line return
tray_background_view_->GetShelf();

Done.

https://codereview.chromium.org/2072023003/diff/1/ash/system/tray/tray_detail...
File ash/system/tray/tray_details_view_unittest.cc (right):

https://codereview.chromium.org/2072023003/diff/1/ash/system/tray/tray_detail...
ash/system/tray/tray_details_view_unittest.cc:197: ui::test::EventGenerator&
generator = GetEventGenerator();
On 2016/06/17 19:58:37, msw wrote:
> Hmm, I thought we discouraged non-const refs, but I guess that's mainly for
> input parameters. Still, it'd be nice if this function returned a pointer.

Yeah, this is bad, but GetEventGenerator() predates me and is used this way in
150 places. Let me know if you want me to fix it and I'll do it in another CL.

[James Cook, 2016-06-17 20:16:46]

The CQ bit was checked by jamescook@chromium.org

[James Cook, 2016-06-17 20:16:47]

The patchset sent to the CQ was uploaded after l-g-t-m from msw@chromium.org
Link to the patchset: https://codereview.chromium.org/2072023003/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2016-06-17 20:17:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2072023003/40001

[commit-bot: I haz the power, 2016-06-17 21:11:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-17 21:11:08]

Failed to apply patch for ash/system/tray/system_tray.cc:
While running git apply --index -3 -p1;
  error: patch failed: ash/system/tray/system_tray.cc:5
  Falling back to three-way merge...
  Applied patch to 'ash/system/tray/system_tray.cc' with conflicts.
  U ash/system/tray/system_tray.cc

Patch:       ash/system/tray/system_tray.cc
Index: ash/system/tray/system_tray.cc
diff --git a/ash/system/tray/system_tray.cc b/ash/system/tray/system_tray.cc
index
1fe6be7da609c4aefabe07b48a3caba7afbb3c5e..0f9adbaee463868318e78c85fab00598c29705fb
100644
--- a/ash/system/tray/system_tray.cc
+++ b/ash/system/tray/system_tray.cc
@@ -5,6 +5,8 @@
 #include "ash/system/tray/system_tray.h"
 
 #include "ash/ash_switches.h"
+#include "ash/common/session/session_state_delegate.h"
+#include "ash/common/shelf/wm_shelf.h"
 #include "ash/common/shelf/wm_shelf_util.h"
 #include "ash/common/shell_window_ids.h"
 #include "ash/common/system/date/tray_date.h"
@@ -14,8 +16,6 @@
 #include "ash/common/system/update/tray_update.h"
 #include "ash/common/wm_shell.h"
 #include "ash/metrics/user_metrics_recorder.h"
-#include "ash/shelf/shelf_layout_manager.h"
-#include "ash/shelf/shelf_util.h"
 #include "ash/shell.h"
 #include "ash/system/audio/tray_audio.h"
 #include "ash/system/cast/tray_cast.h"
@@ -31,7 +31,6 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/timer/timer.h"
 #include "grit/ash_strings.h"
-#include "ui/aura/window_event_dispatcher.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/compositor/layer.h"
 #include "ui/display/display.h"
@@ -531,7 +530,7 @@ void SystemTray::ShowItems(const
std::vector<SystemTrayItem*>& items,
   UpdateNotificationBubble();  // State changed, re-create notifications.
   if (!notification_bubble_)
     UpdateWebNotifications();
-  GetShelfLayoutManager()->UpdateAutoHideState();
+  GetShelf()->UpdateAutoHideState();
 
   // When we show the system menu in our alternate shelf layout, we need to
   // tint the background.
@@ -651,7 +650,7 @@ void SystemTray::HideBubbleWithView(const TrayBubbleView*
bubble_view) {
   if (system_bubble_.get() && bubble_view == system_bubble_->bubble_view()) {
     DestroySystemBubble();
     UpdateNotificationBubble();  // State changed, re-create notifications.
-    GetShelfLayoutManager()->UpdateAutoHideState();
+    GetShelf()->UpdateAutoHideState();
   } else if (notification_bubble_.get() &&
              bubble_view == notification_bubble_->bubble_view()) {
     DestroyNotificationBubble();

[James Cook, 2016-06-17 21:19:15]

The CQ bit was checked by jamescook@chromium.org

[James Cook, 2016-06-17 21:19:16]

The patchset sent to the CQ was uploaded after l-g-t-m from msw@chromium.org
Link to the patchset: https://codereview.chromium.org/2072023003/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2016-06-17 21:19:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2072023003/60001

[commit-bot: I haz the power, 2016-06-17 22:10:18]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-17 22:12:34]

Description was changed from

==========
mash: Break ash system tray dependencies on ash::ShelfWidget

This is a step towards converting the system tray to wm common types, as well
as a step toward hiding the internal details of the shelf implementation from
external users. Long term, all access to shelf state should go via WmShelf.

* Plumb WmShelf into StatusAreaWidget.
* Remove ShelfWidget usage from //ash/system.
* Remove unnecessary includes.

BUG=615502,619636
TEST=ash_unittests
==========

to

==========
mash: Break ash system tray dependencies on ash::ShelfWidget

This is a step towards converting the system tray to wm common types, as well
as a step toward hiding the internal details of the shelf implementation from
external users. Long term, all access to shelf state should go via WmShelf.

* Plumb WmShelf into StatusAreaWidget.
* Remove ShelfWidget usage from //ash/system.
* Remove unnecessary includes.

BUG=615502,619636
TEST=ash_unittests

Committed: https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee
Cr-Commit-Position: refs/heads/master@{#400520}
==========

[commit-bot: I haz the power, 2016-06-17 22:12:35]

Patchset 4 (id:??) landed as
https://crrev.com/aada0db95278da7d4d74bae5d6dfb4480d5b3bee
Cr-Commit-Position: refs/heads/master@{#400520}