Send notification to users upon receiving sms messages

ash/common/shelf/shelf_widget.cc

 // Copyright 2012 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 "ash/common/shelf/shelf_widget.h"
 
 #include "ash/common/focus_cycler.h"
 #include "ash/common/material_design/material_design_controller.h"
 #include "ash/common/session/session_state_delegate.h"
 #include "ash/common/shelf/app_list_button.h"
 #include "ash/common/shelf/shelf_background_animator_observer.h"
 #include "ash/common/shelf/shelf_constants.h"
 #include "ash/common/shelf/shelf_delegate.h"
 #include "ash/common/shelf/shelf_layout_manager.h"
 #include "ash/common/shelf/shelf_view.h"
 #include "ash/common/shelf/wm_shelf.h"
 #include "ash/common/shelf/wm_shelf_util.h"
+#include "ash/common/system/chromeos/network/sms_observer.h"
 #include "ash/common/system/status_area_layout_manager.h"
 #include "ash/common/system/status_area_widget.h"
 #include "ash/common/wm_lookup.h"
 #include "ash/common/wm_shell.h"
 #include "ash/common/wm_window.h"
 #include "ash/common/wm_window_property.h"
 #include "ash/root_window_controller.h"
 #include "base/memory/ptr_util.h"
 #include "grit/ash_resources.h"
 #include "ui/base/resource/resource_bundle.h"
@@ skipping 184 matching lines @@
   asset_background_alpha_ = alpha;
   SchedulePaint();
 }
 
 ShelfWidget::ShelfWidget(WmWindow* shelf_container, WmShelf* wm_shelf)
     : wm_shelf_(wm_shelf),
       status_area_widget_(nullptr),
       delegate_view_(new DelegateView(wm_shelf, this)),
       shelf_view_(nullptr),
       background_animator_(SHELF_BACKGROUND_DEFAULT, wm_shelf_),
+      sms_observer_(nullptr),
       activating_as_fallback_(false) {
   background_animator_.AddObserver(this);
   background_animator_.AddObserver(delegate_view_);
 
   views::Widget::InitParams params(
       views::Widget::InitParams::TYPE_WINDOW_FRAMELESS);
   params.name = "ShelfWidget";
   params.opacity = views::Widget::InitParams::TRANSLUCENT_WINDOW;
   params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
   params.delegate = delegate_view_;
@@ skipping 32 matching lines @@
   DCHECK(!status_area_widget_);
   // TODO(jamescook): Move ownership to RootWindowController.
   status_area_widget_ = new StatusAreaWidget(status_container, wm_shelf_);
   status_area_widget_->CreateTrayViews();
   if (WmShell::Get()->GetSessionStateDelegate()->IsActiveUserSessionStarted())
     status_area_widget_->Show();
   WmShell::Get()->focus_cycler()->AddWidget(status_area_widget_);
   background_animator_.AddObserver(status_area_widget_);
   status_container->SetLayoutManager(
       base::MakeUnique<StatusAreaLayoutManager>(this));
+  sms_observer_ = new SmsObserver();

[tdanderson, 2017/01/26 22:19:06]

Is there a specific reason why you chose to instantiate |sms_observer_| here
rather than in the ctor or initializer list?

[yiyix, 2017/02/02 20:43:55]

sorry, it's a mistake. 

[tdanderson, 2017/02/07 00:15:44]

Acknowledged.

 }
 
 void ShelfWidget::SetPaintsBackground(
     ShelfBackgroundType background_type,
     BackgroundAnimatorChangeType change_type) {
   background_animator_.PaintBackground(background_type, change_type);
 }
 
 ShelfBackgroundType ShelfWidget::GetBackgroundType() const {
   return background_animator_.target_background_type();
@@ skipping 94 matching lines @@
   if (shelf_layout_manager_)
     shelf_layout_manager_->PrepareForShutdown();
 
   if (status_area_widget_) {
     background_animator_.RemoveObserver(status_area_widget_);
     WmShell::Get()->focus_cycler()->RemoveWidget(status_area_widget_);
     status_area_widget_->Shutdown();
     status_area_widget_ = nullptr;
   }
 
+  sms_observer_->Shutdown();

[tdanderson, 2017/01/26 22:19:06]

Is there a reason why you need to introduce a Shutdown() method and call it at
this specific place? It seems if you make ShelfWidget own SmsObserver, then
de-registering the observer can be done in the SmsObserver dtor rather than
needing a separate Shutdown() method.

If left as-is, I suggest guarding against a null |sms_observer_|.

[yiyix, 2017/02/02 20:43:55]

I originally didn't create the shutdown method, then i keep receive this failure
as I exit chrome in my local build:
[105378:105378:0202/152205.133528:FATAL:observer_list.h(338)] Check failed:
ObserverListBase<ObserverType>::size() == 0U (1 vs. 0)

From the stack trace, I believe it calls shutdown first then it calls the dtor.
So i added this line to de-register sms_observer properly. I have added the
check as you have recommended.

[yiyix, 2017/02/02 21:24:19]

Sorry, after analyzing again, I understand that the dtor is not called. So I
updated it to unique_ptr and removed the unnecessary shutdown method.

[tdanderson, 2017/02/07 00:15:45]

Thanks for looking into this in more detail, what you have in PS2 looks good!

   CloseNow();
 }
 
 void ShelfWidget::UpdateIconPositionForPanel(WmWindow* panel) {
   if (!shelf_view_)
     return;
 
   WmWindow* shelf_window = WmLookup::Get()->GetWindowForWidget(this);
   shelf_view_->UpdatePanelIconPosition(
       panel->GetIntProperty(WmWindowProperty::SHELF_ID),
@@ skipping 38 matching lines @@
   if (shelf_view_)
     shelf_view_->UpdateShelfItemBackground(alpha);
 }
 
 void ShelfWidget::WillDeleteShelfLayoutManager() {
   shelf_layout_manager_->RemoveObserver(this);
   shelf_layout_manager_ = nullptr;
 }
 
 }  // namespace ash