Synced Notification Sync Change Processor

chrome/browser/notifier/chrome_notifier_delegate.h

+// Copyright (c) 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.
+
+// ChromeNotifierDelegate is a NotificationDelegate which catches
+// responses from the NotificationUIManager when a notification
+// has been closed.

[dcheng, 2013/01/18 21:56:13]

Typical convention is to place class comments above the class (in this case,
after line 13). Also, what is this class going to do eventually? It certainly
seems like it handles more things than just Close(), so how come we don't need
to implement any of the other methods?

[Pete Williamson, 2013/01/23 01:45:55]

Comment moved.  Actually, we don't do the display, we only need the closed
notification so we can change the "read" flag on the notification.

+
+#ifndef CHROME_BROWSER_NOTIFIER_CHROME_NOTIFIER_DELEGATE_H_
+#define CHROME_BROWSER_NOTIFIER_CHROME_NOTIFIER_DELEGATE_H_
+
+#include "chrome/browser/notifications/notification_delegate.h"
+
+class ChromeNotifierDelegate : public NotificationDelegate {
+ public:
+  explicit ChromeNotifierDelegate(const std::string& id) :
+      id_(id) {}

[dcheng, 2013/01/18 21:56:13]

Please move this to the .cc.

[Pete Williamson, 2013/01/23 01:45:55]

Done.

+
+  // NotificationDelegate interface.
+  virtual void Display() OVERRIDE {}
+  virtual void Error() OVERRIDE {}
+  virtual void Close(bool by_user) OVERRIDE;
+  virtual void Click() OVERRIDE {}
+  virtual std::string id() const OVERRIDE { return id_; }
+  virtual content::RenderViewHost* GetRenderViewHost() const OVERRIDE {
+    return NULL;
+}

[dcheng, 2013/01/18 21:56:13]

Fix indent. Or just move all the empty bodies to the .cc.

[Pete Williamson, 2013/01/23 01:45:55]

indent fixed

+
+ private:
+  virtual ~ChromeNotifierDelegate() {}

[dcheng, 2013/01/18 21:56:13]

Please move this to the .cc.

+
+  std::string id_;

[dcheng, 2013/01/18 21:56:13]

const? This Or can this change?

[Pete Williamson, 2013/01/23 01:45:55]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(ChromeNotifierDelegate);
+};
+
+#endif  // CHROME_BROWSER_NOTIFIER_CHROME_NOTIFIER_DELEGATE_H_