Adding an experimental app notification API.

chrome/browser/extensions/extension_app_api.cc

+// Copyright (c) 2011 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 "chrome/browser/extensions/extension_app_api.h"
+
+#include "base/stl_util-inl.h"
+#include "base/values.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/common/extensions/extension.h"
+#include "chrome/common/render_messages.h"
+#include "content/common/notification_service.h"
+#include "content/common/notification_type.h"
+
+
+const char kBodyTextKey[] = "bodyText";
+const char kIconDataKey[] = "iconData";
+const char kLinkTextKey[] = "linkText";
+const char kLinkUrlKey[] = "linkUrl";
+const char kTitleKey[] = "title";
+
+AppNotificationManager::AppNotificationManager() {
+  registrar_.Add(this,
+                 NotificationType::EXTENSION_UNINSTALLED,
+                 NotificationService::AllSources());
+}
+
+AppNotificationManager::~AppNotificationManager() {}
+
+void AppNotificationManager::Add(AppNotification* item) {
+  CHECK(!item->extension_id.empty());
+  NotificationMap::iterator found = items_.find(item->extension_id);

[Finnur, 2011/06/20 12:21:46]

It would probably help readability a bit to rename |items_| to
|app_notifications_| (or something). |items_| is a bit generic.

[asargent_no_longer_on_chrome, 2011/06/21 18:10:11]

Good point. Changed to "notifications_".

+  if (found == items_.end()) {
+    items_[item->extension_id] = AppNotificationList();

[Finnur, 2011/06/20 12:21:46]

AppNotificationList()? Where does that come from? I can't find it elsewhere in
this patch. Could there be a file missing since the CL didn't go through a try?

[asargent_no_longer_on_chrome, 2011/06/21 18:10:11]

It's in extension_app_api.h, just before the start of the declaration of the
AppNotificationManager class. I added a comment to the top of it.

+    found = items_.find(item->extension_id);
+  }
+  CHECK(found != items_.end());
+  AppNotificationList& list = (*found).second;
+  list.push_back(linked_ptr<AppNotification>(item));
+}
+
+const AppNotificationList* AppNotificationManager::GetAll(
+    const std::string& extension_id) {
+  if (ContainsKey(items_, extension_id))
+    return &items_[extension_id];
+  return NULL;
+}
+
+void AppNotificationManager::ClearAll(const std::string& extension_id) {
+  NotificationMap::iterator found = items_.find(extension_id);
+  if (found != items_.end())
+    items_.erase(found);
+}
+
+void AppNotificationManager::Observe(NotificationType type,
+                                     const NotificationSource& source,
+                                     const NotificationDetails& details) {
+  CHECK(type == NotificationType::EXTENSION_UNINSTALLED);
+  const std::string& id =
+      Details<UninstalledExtensionInfo>(details)->extension_id;
+  ClearAll(id);
+}
+
+

[Finnur, 2011/06/20 12:21:46]

nit: extra line break

[asargent_no_longer_on_chrome, 2011/06/21 18:10:11]

Done.

+bool AppNotifyFunction::RunImpl() {
+  DictionaryValue* details;
+  EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(0, &details));
+  EXTENSION_FUNCTION_VALIDATE(details != NULL);
+
+  scoped_ptr<AppNotification> item(new AppNotification());
+  item->extension_id = extension_id();
+
+  if (details->HasKey(kTitleKey))
+    EXTENSION_FUNCTION_VALIDATE(details->GetString(kTitleKey, &item->title));
+
+  if (details->HasKey(kBodyTextKey))
+    EXTENSION_FUNCTION_VALIDATE(details->GetString(kBodyTextKey, &item->body));
+
+  if (details->HasKey(kLinkUrlKey)) {
+    std::string link_url;
+    EXTENSION_FUNCTION_VALIDATE(details->GetString(kLinkUrlKey, &link_url));
+    item->linkUrl = GURL(link_url);
+    if (!item->linkUrl.is_valid()) {
+      error_ = "Invalid url: " + link_url;
+      return false;
+    }
+  }
+
+  if (details->HasKey(kLinkTextKey))
+    EXTENSION_FUNCTION_VALIDATE(details->GetString(kLinkTextKey,
+                                                   &item->linkText));

[Finnur, 2011/06/20 12:21:46]

I would move this block into line 88 (within the block).
Also, if details->HasKey(kLinkTextKey) == false then set the linkText to be =
link_url.

[asargent_no_longer_on_chrome, 2011/06/21 18:10:11]

Good point on moving it into the block above - I've done that.

As for defaulting the link text to the URL if unspecified, I wonder: would it be
better to just do that in the NTP code if the link text is the empty string?
That way we could have some sort of default text like "link" or something which
we might decide works better than potentially long urls. 

[Finnur, 2011/06/22 10:40:23]

I think we should address it at this funnel rather than rely on consumers to do
the right thing. I see two possibilities why a link text is empty:

1) Left out by mistake
2) On purpose because the link is short and self-describing.

I suspect a default of 'link' is probably no better than showing the URL for
case 1 and annoying for case 2. But how about simply aborting if URL is provided
but the description is blank?



On 2011/06/21 18:10:11, Antony Sargent wrote:

[asargent_no_longer_on_chrome, 2011/06/22 17:36:36]

Ok, that seems reasonable. We can certainly change this later if we decide it's
annoying from a developer perspective.

+
+  if (details->HasKey(kIconDataKey)) {
+    BinaryValue* binary = NULL;
+    EXTENSION_FUNCTION_VALIDATE(details->GetBinary(kIconDataKey, &binary));
+    IPC::Message bitmap_pickle(binary->GetBuffer(), binary->GetSize());
+    void* iter = NULL;
+    SkBitmap bitmap;
+    EXTENSION_FUNCTION_VALIDATE(
+        IPC::ReadParam(&bitmap_pickle, &iter, &bitmap));
+    // TODO(asargent) - use the bitmap to set the NTP icon!
+  }
+
+  AppNotificationManager* manager =
+      profile()->GetExtensionService()->app_notification_manager();
+
+  manager->Add(item.release());
+
+  NotificationService::current()->Notify(
+      NotificationType::APP_NOTIFICATION_STATE_CHANGED,
+      Source<Profile>(profile_),
+      NotificationService::NoDetails());
+
+  return true;
+}
+
+bool AppClearAllNotificationsFunction::RunImpl() {
+  AppNotificationManager* manager =
+      profile()->GetExtensionService()->app_notification_manager();
+  manager->ClearAll(extension_id());
+  NotificationService::current()->Notify(
+      NotificationType::APP_NOTIFICATION_STATE_CHANGED,
+      Source<Profile>(profile_),
+      NotificationService::NoDetails());
+  return true;
+}