Skip to content

Commit

Permalink
This code cleanup is part of a readability review.
Browse files Browse the repository at this point in the history 
Review Bug 12897471

Review URL: https://codereview.chromium.org/155353002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@250507 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
robliao@chromium.org committed Feb 11, 2014
1 parent 955c068 commit 0ff5948
Show file tree
Hide file tree
Showing 3 changed files with 158 additions and 124 deletions.
66 changes: 35 additions & 31 deletions chrome/browser/notifications/extension_welcome_notification.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,19 @@
#include "ui/message_center/notification_delegate.h"
#include "ui/message_center/notification_types.h"

const unsigned int ExtensionWelcomeNotification::kRequestedShowTimeDays = 1;
const int ExtensionWelcomeNotification::kRequestedShowTimeDays = 1;

namespace {

class NotificationCallbacks
: public message_center::NotificationDelegate {
public:
NotificationCallbacks(
Profile* profile,
ExtensionWelcomeNotification::Delegate* delegate)
: profile_(profile),
delegate_(delegate) {}
delegate_(delegate) {
}

// Overridden from NotificationDelegate:
virtual void Display() OVERRIDE {}
Expand All @@ -54,7 +56,7 @@ class NotificationCallbacks

virtual void Click() OVERRIDE {}
virtual void ButtonClick(int index) OVERRIDE {
DCHECK(index == 0);
DCHECK_EQ(index, 0);
OpenNotificationLearnMoreTab();
}

Expand All @@ -79,7 +81,7 @@ class NotificationCallbacks
Profile* const profile_;

// Weak ref owned by ExtensionWelcomeNotification.
ExtensionWelcomeNotification::Delegate* delegate_;
ExtensionWelcomeNotification::Delegate* const delegate_;

DISALLOW_COPY_AND_ASSIGN(NotificationCallbacks);
};
Expand All @@ -102,16 +104,16 @@ class DefaultDelegate : public ExtensionWelcomeNotification::Delegate {
base::MessageLoop::current()->PostTask(from_here, task);
}

private:
DISALLOW_COPY_AND_ASSIGN(DefaultDelegate);
private:
DISALLOW_COPY_AND_ASSIGN(DefaultDelegate);
};

} // namespace

ExtensionWelcomeNotification::ExtensionWelcomeNotification(
const std::string& extension_id,
Profile* profile,
ExtensionWelcomeNotification::Delegate* delegate)
Profile* const profile,
ExtensionWelcomeNotification::Delegate* const delegate)
: notifier_id_(message_center::NotifierId::APPLICATION, extension_id),
profile_(profile),
delegate_(delegate) {
Expand All @@ -123,18 +125,18 @@ ExtensionWelcomeNotification::ExtensionWelcomeNotification(
base::Unretained(this)));
}

// Static
// static
scoped_ptr<ExtensionWelcomeNotification> ExtensionWelcomeNotification::Create(
const std::string& extension_id,
Profile* profile) {
const std::string& extension_id,
Profile* const profile) {
return Create(extension_id, profile, new DefaultDelegate()).Pass();
}

// Static
// static
scoped_ptr<ExtensionWelcomeNotification> ExtensionWelcomeNotification::Create(
const std::string& extension_id,
Profile* profile,
Delegate* delegate) {
Profile* const profile,
Delegate* const delegate) {
return scoped_ptr<ExtensionWelcomeNotification>(
new ExtensionWelcomeNotification(extension_id, profile, delegate)).Pass();
}
Expand All @@ -150,7 +152,7 @@ ExtensionWelcomeNotification::~ExtensionWelcomeNotification() {

void ExtensionWelcomeNotification::OnIsSyncingChanged() {
DCHECK(delayed_notification_);
PrefServiceSyncable* pref_service_syncable =
PrefServiceSyncable* const pref_service_syncable =
PrefServiceSyncable::FromProfile(profile_);
if (pref_service_syncable->IsSyncing()) {
pref_service_syncable->RemoveObserver(this);
Expand All @@ -163,12 +165,12 @@ void ExtensionWelcomeNotification::OnIsSyncingChanged() {
void ExtensionWelcomeNotification::ShowWelcomeNotificationIfNecessary(
const Notification& notification) {
if ((notification.notifier_id() == notifier_id_) && !delayed_notification_) {
PrefServiceSyncable* pref_service_syncable =
PrefServiceSyncable* const pref_service_syncable =
PrefServiceSyncable::FromProfile(profile_);
if (pref_service_syncable->IsSyncing()) {
PrefService* pref_service = profile_->GetPrefs();
PrefService* const pref_service = profile_->GetPrefs();
if (!pref_service->GetBoolean(prefs::kWelcomeNotificationDismissed)) {
PopUpRequest pop_up_request =
const PopUpRequest pop_up_request =
pref_service->GetBoolean(
prefs::kWelcomeNotificationPreviouslyPoppedUp)
? POP_UP_HIDDEN
Expand All @@ -192,7 +194,7 @@ void ExtensionWelcomeNotification::ShowWelcomeNotificationIfNecessary(
}
}

// Static
// static
void ExtensionWelcomeNotification::RegisterProfilePrefs(
user_prefs::PrefRegistrySyncable* prefs) {
prefs->RegisterBooleanPref(prefs::kWelcomeNotificationDismissed,
Expand All @@ -207,13 +209,13 @@ void ExtensionWelcomeNotification::RegisterProfilePrefs(
}

message_center::MessageCenter*
ExtensionWelcomeNotification::GetMessageCenter() {
ExtensionWelcomeNotification::GetMessageCenter() const {
return delegate_->GetMessageCenter();
}

void ExtensionWelcomeNotification::ShowWelcomeNotification(
const base::string16& display_source,
PopUpRequest pop_up_request) {
const PopUpRequest pop_up_request) {
message_center::ButtonInfo learn_more(
l10n_util::GetStringUTF16(IDS_NOTIFICATION_WELCOME_BUTTON_LEARN_MORE));
learn_more.icon = ui::ResourceBundle::GetSharedInstance().GetImageNamed(
Expand Down Expand Up @@ -294,25 +296,27 @@ void ExtensionWelcomeNotification::ExpireWelcomeNotification() {
HideWelcomeNotification();
}

base::Time ExtensionWelcomeNotification::GetExpirationTimestamp() {
PrefService* pref_service = profile_->GetPrefs();
int64 expiration_timestamp =
base::Time ExtensionWelcomeNotification::GetExpirationTimestamp() const {
PrefService* const pref_service = profile_->GetPrefs();
const int64 expiration_timestamp =
pref_service->GetInt64(prefs::kWelcomeNotificationExpirationTimestamp);
return (expiration_timestamp == 0) ?
base::Time() :
base::Time::FromInternalValue(expiration_timestamp);
return (expiration_timestamp == 0)
? base::Time()
: base::Time::FromInternalValue(expiration_timestamp);
}

void ExtensionWelcomeNotification::SetExpirationTimestampFromNow() {
PrefService* pref_service = profile_->GetPrefs();
PrefService* const pref_service = profile_->GetPrefs();
pref_service->SetInt64(
prefs::kWelcomeNotificationExpirationTimestamp,
(delegate_->GetCurrentTime() +
base::TimeDelta::FromDays(kRequestedShowTimeDays)).ToInternalValue());
}

bool ExtensionWelcomeNotification::IsWelcomeNotificationExpired() {
base::Time expiration_timestamp = GetExpirationTimestamp();
bool ExtensionWelcomeNotification::IsWelcomeNotificationExpired() const {
const base::Time expiration_timestamp = GetExpirationTimestamp();
return !expiration_timestamp.is_null() &&
(expiration_timestamp <= delegate_->GetCurrentTime());
(expiration_timestamp <= delegate_->GetCurrentTime());
}

// C++ Readability Review Change Trigger
53 changes: 32 additions & 21 deletions chrome/browser/notifications/extension_welcome_notification.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,61 +38,67 @@ class Profile;
// connectivity since it relies on synced preferences to work. This is generally
// fine since the current consumers on the welcome notification also presume
// network connectivity.
// This class expects to be created and called from the UI thread.
class ExtensionWelcomeNotification : public PrefServiceSyncableObserver {
public:
// Requested time from showing the welcome notification to expiration.
static const unsigned int kRequestedShowTimeDays;

// Allows for overriding global calls.
class Delegate {
public:
Delegate() {}
virtual ~Delegate() {}
virtual message_center::MessageCenter* GetMessageCenter() = 0;
virtual base::Time GetCurrentTime() = 0;
virtual void PostTask(
const tracked_objects::Location& from_here,
const base::Closure& task) = 0;
private:
DISALLOW_COPY_AND_ASSIGN(Delegate);
};

// Requested time from showing the welcome notification to expiration.
static const int kRequestedShowTimeDays;

virtual ~ExtensionWelcomeNotification();

// To workaround the lack of delegating constructors prior to C++11, we use
// static Create methods.
// Creates an ExtensionWelcomeNotification with the default delegate.
// Creates an ExtensionWelcomeNotification owned by the specified
// extension id with the default delegate.
static scoped_ptr<ExtensionWelcomeNotification> Create(
const std::string& extension_id,
Profile* profile);
Profile* const profile);

// Creates an ExtensionWelcomeNotification with the specified delegate.
// Creates an ExtensionWelcomeNotification owned by the specified
// extension id with the specified delegate.
static scoped_ptr<ExtensionWelcomeNotification> Create(
const std::string& extension_id,
Profile* profile,
Delegate* delegate);

virtual ~ExtensionWelcomeNotification();
Profile* const profile,
Delegate* const delegate);

// PrefServiceSyncableObserver
virtual void OnIsSyncingChanged() OVERRIDE;

// Adds in a the welcome notification if required for components built
// Adds in the welcome notification if required for components built
// into Chrome that show notifications like Chrome Now.
void ShowWelcomeNotificationIfNecessary(const Notification& notification);

// Handles Preference Registeration for the Welcome Notification.
// Handles Preference Registration for the Welcome Notification.
static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* prefs);

private:
enum PopUpRequest { POP_UP_HIDDEN = 0, POP_UP_SHOWN = 1, };

ExtensionWelcomeNotification(
const std::string& extension_id,
Profile* profile,
ExtensionWelcomeNotification::Delegate* delegate);

enum PopUpRequest { POP_UP_HIDDEN = 0, POP_UP_SHOWN = 1, };
Profile* const profile,
ExtensionWelcomeNotification::Delegate* const delegate);

// Gets the message center from the delegate.
message_center::MessageCenter* GetMessageCenter();
message_center::MessageCenter* GetMessageCenter() const;

// Unconditionally shows the welcome notification.
void ShowWelcomeNotification(const base::string16& display_source,
PopUpRequest pop_up_request);
const PopUpRequest pop_up_request);

// Hides the welcome notification.
void HideWelcomeNotification();
Expand All @@ -110,13 +116,13 @@ class ExtensionWelcomeNotification : public PrefServiceSyncableObserver {
void ExpireWelcomeNotification();

// Gets the expiration timestamp or a null time is there is none.
base::Time GetExpirationTimestamp();
base::Time GetExpirationTimestamp() const;

// Sets the expiration timestamp from now.
void SetExpirationTimestampFromNow();

// True if the welcome notification has expired, false otherwise.
bool IsWelcomeNotificationExpired();
bool IsWelcomeNotificationExpired() const;

// Prefs listener for welcome_notification_dismissed.
BooleanPrefMember welcome_notification_dismissed_pref_;
Expand All @@ -142,8 +148,13 @@ class ExtensionWelcomeNotification : public PrefServiceSyncableObserver {
scoped_ptr<base::OneShotTimer<ExtensionWelcomeNotification> >
expiration_timer_;

// Delegate for global calls.
// Delegate for Chrome global calls like base::Time::GetTime() for
// testability.
scoped_ptr<Delegate> delegate_;

DISALLOW_COPY_AND_ASSIGN(ExtensionWelcomeNotification);
};

#endif // CHROME_BROWSER_NOTIFICATIONS_EXTENSION_WELCOME_NOTIFICATION_H_

// C++ Readability Review Change Trigger
Loading

0 comments on commit 0ff5948

Please sign in to comment.