Fix window.close() for page action popups.

Convert mappy to a page action popup.
Also fix a bug where disabling an extension with a page action didn't cause the icon to hide.

BUG=27519
TEST=none

Review URL: http://codereview.chromium.org/414019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@32616 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 105 insertions, 37 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 55eca73..805d726 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -144,6 +144,8 @@ Browser::Browser(Type type, Profile* profile)
                  NotificationService::AllSources());
   registrar_.Add(this, NotificationType::EXTENSION_UNLOADED,
                  NotificationService::AllSources());
+  registrar_.Add(this, NotificationType::EXTENSION_UNLOADED_DISABLED,
+                 NotificationService::AllSources());
   registrar_.Add(this, NotificationType::EXTENSION_PROCESS_CRASHED,
                  NotificationService::AllSources());
   registrar_.Add(this, NotificationType::BROWSER_THEME_CHANGED,
@@ -2296,8 +2298,9 @@ void Browser::Observe(NotificationType type,
       break;
     }
 
-    case NotificationType::EXTENSION_UNLOADED: {
-      window()->GetLocationBar()->InvalidatePageActions();
+    case NotificationType::EXTENSION_UNLOADED:
+    case NotificationType::EXTENSION_UNLOADED_DISABLED: {
+      window()->GetLocationBar()->UpdatePageActions();
 
       // Close any tabs from the unloaded extension.
       Extension* extension = Details<Extension>(details).ptr();
diff --git a/chrome/browser/views/location_bar_view.cc b/chrome/browser/views/location_bar_view.cc
index a0033b6..15ac714 100644
--- a/chrome/browser/views/location_bar_view.cc
+++ b/chrome/browser/views/location_bar_view.cc
@@ -1315,6 +1315,9 @@ LocationBarView::PageActionImageView::PageActionImageView(
         gfx::Size(Extension::kPageActionIconMaxSize,
                   Extension::kPageActionIconMaxSize));
   }
+
+  registrar_.Add(this, NotificationType::EXTENSION_HOST_VIEW_SHOULD_CLOSE,
+                 Source<Profile>(profile_));
 }
 
 LocationBarView::PageActionImageView::~PageActionImageView() {
@@ -1335,12 +1338,19 @@ void LocationBarView::PageActionImageView::ExecuteAction(int button) {
     if (!browser)
       browser = BrowserList::FindBrowserWithProfile(profile_);
 
+    bool popup_showing = popup_ != NULL;
+
     // Always hide the current popup. Only one popup at a time.
     HidePopup();
 
+    // If we were already showing, then treat this click as a dismiss.
+    if (popup_showing)
+      return;
+
+    View* parent = GetParent();
     gfx::Point origin;
-    View::ConvertPointToScreen(this, &origin);
-    gfx::Rect rect = bounds();
+    View::ConvertPointToScreen(parent, &origin);
+    gfx::Rect rect = parent->bounds();
     rect.set_x(origin.x());
     rect.set_y(origin.y());
     popup_ = ExtensionPopup::Show(page_action_->popup_url(), browser, rect,
@@ -1481,6 +1491,24 @@ void LocationBarView::PageActionImageView::HidePopup() {
   delete closing_popup;
 }
 
+void LocationBarView::PageActionImageView::Observe(
+    NotificationType type,
+    const NotificationSource& source,
+    const NotificationDetails& details) {
+  switch (type.value) {
+    case NotificationType::EXTENSION_HOST_VIEW_SHOULD_CLOSE:
+      // If we aren't the host of the popup, then disregard the notification.
+      if (!popup_ || Details<ExtensionHost>(popup_->host()) != details)
+        return;
+
+      HidePopup();
+      break;
+    default:
+      NOTREACHED() << "Unexpected notification";
+      break;
+  }
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // LocationBarView, LocationBar implementation:
 
diff --git a/chrome/browser/views/location_bar_view.h b/chrome/browser/views/location_bar_view.h
index 91ca81b..64c937d 100644
--- a/chrome/browser/views/location_bar_view.h
+++ b/chrome/browser/views/location_bar_view.h
@@ -19,6 +19,8 @@
 #include "chrome/browser/toolbar_model.h"
 #include "chrome/browser/views/browser_bubble.h"
 #include "chrome/browser/views/info_bubble.h"
+#include "chrome/common/notification_observer.h"
+#include "chrome/common/notification_registrar.h"
 #include "views/controls/image_view.h"
 #include "views/controls/label.h"
 #include "views/controls/native/native_view_host.h"
@@ -363,6 +365,7 @@ class LocationBarView : public LocationBar,
   // and notify the extension when the icon is clicked.
   class PageActionImageView : public LocationBarImageView,
                               public ImageLoadingTracker::Observer,
+                              public NotificationObserver,
                               public BrowserBubble::Delegate {
    public:
     PageActionImageView(LocationBarView* owner,
@@ -405,6 +408,11 @@ class LocationBarView : public LocationBar,
     // Hides the active popup, if there is one.
     void HidePopup();
 
+    // Overridden from NotificationObserver:
+    virtual void Observe(NotificationType type,
+                         const NotificationSource& source,
+                         const NotificationDetails& details);
+
     // The location bar view that owns us.
     LocationBarView* owner_;
 
@@ -441,6 +449,8 @@ class LocationBarView : public LocationBar,
 
     ScopedRunnableMethodFactory<PageActionImageView> method_factory_;
 
+    NotificationRegistrar registrar_;
+
     DISALLOW_COPY_AND_ASSIGN(PageActionImageView);
   };
   friend class PageActionImageView;
diff --git a/chrome/common/extensions/docs/examples/extensions/mappy/background.html b/chrome/common/extensions/docs/examples/extensions/mappy/background.html
new file mode 100755
index 0000000..ff6f7c9
--- /dev/null
+++ b/chrome/common/extensions/docs/examples/extensions/mappy/background.html
@@ -0,0 +1,42 @@
+<script>

+// Global accessor that the popup uses.

+var addresses = {};

+var selectedAddress = null;

+var selectedId = null;

+

+function updateAddress(tabId) {

+  chrome.tabs.sendRequest(tabId, {}, function(address) {

+    addresses[tabId] = address;

+    if (!address) {

+      chrome.pageAction.hide(tabId);

+    } else {

+      chrome.pageAction.show(tabId);

+      if (selectedId == tabId) {

+        updateSelected(tabId);

+      }

+    }

+  });

+}

+

+function updateSelected(tabId) {

+  selectedAddress = addresses[tabId];

+  if (selectedAddress)

+    chrome.pageAction.setTitle({tabId:tabId, title:selectedAddress});

+}

+

+chrome.tabs.onUpdated.addListener(function(tabId, change, tab) {

+  if (change.status == "complete") {

+    updateAddress(tabId);

+  }

+});

+

+chrome.tabs.onSelectionChanged.addListener(function(tabId, info) {

+  selectedId = tabId;

+  updateSelected(tabId);

+});

+

+// Ensure the current selected tab is set up.

+chrome.tabs.getSelected(null, function(tab) {

+  updateAddress(tab.id);

+});

+</script>

diff --git a/chrome/common/extensions/docs/examples/extensions/mappy/manifest.json b/chrome/common/extensions/docs/examples/extensions/mappy/manifest.json
index ce2eccc..6ab77c7 100755
--- a/chrome/common/extensions/docs/examples/extensions/mappy/manifest.json
+++ b/chrome/common/extensions/docs/examples/extensions/mappy/manifest.json
@@ -3,6 +3,7 @@
   "version": "0.6",
   "description": "Finds addresses in the web page you're on and pops up a map window.",
   "icons": { "128": "icon.png" },
+  "background_page": "background.html",
   "content_scripts": [
     { "matches": ["http://*/*"], "js": ["mappy_content_script.js"] }
   ],
@@ -10,9 +11,9 @@
     "tabs",
     "http://maps.google.com/*"
   ],
-  "browser_action": {
-      "name": "Display Map",
+  "page_action": {
+      "default_name": "Display Map",
       "default_icon": "marker.png",
-      "popup": { "path": "popup.html" }
+      "popup": "popup.html"
   }
 }
diff --git a/chrome/common/extensions/docs/examples/extensions/mappy/mappy_content_script.js b/chrome/common/extensions/docs/examples/extensions/mappy/mappy_content_script.js
index 7e17af7..c4c5fba 100755
--- a/chrome/common/extensions/docs/examples/extensions/mappy/mappy_content_script.js
+++ b/chrome/common/extensions/docs/examples/extensions/mappy/mappy_content_script.js
@@ -1,18 +1,13 @@
-// find map on demand
-
-console.log("mappy_content_script.js loaded");
-
-var maps_key = "ABQIAAAATfHumDbW3OmRByfquHd3SRTRERdeAiwZ9EeJWta3L_JZVS0bOBRQeZgr4K0xyVKzUdnnuFl8X9PX0w";
-
-chrome.extension.onConnect.addListener(function(port) {
-  //console.log("extension connected");
-  port.onMessage.addListener(function(data) {
-    //console.log("extension sent message");
-    findAddress(port);
+// The background page is asking us to find an address on the page.
+if (window == top) {
+  chrome.extension.onRequest.addListener(function(req, sender, sendResponse) {
+    sendResponse(findAddress());
   });
-});
+}
 
-var findAddress = function(port) {
+// Search the text nodes for a US-style mailing address.
+// Return null if none is found.
+var findAddress = function() {
   var found;
   var re = /(\d+\s+[':.,\s\w]*,\s*[A-Za-z]+\s*\d{5}(-\d{4})?)/m;
   var node = document.body;
@@ -42,14 +37,12 @@ var findAddress = function(port) {
     if (match && match.length) {
       console.log("found: " + match[0]);
       var trim = /\s{2,}/g;
-      var map = match[0].replace(trim, " ");
-      port.postMessage({message:"map", values:[map]});
+      return match[0].replace(trim, " ");
     } else {
-      console.log("found bad " + found.textContent);
+      console.log("bad initial match: " + found.textContent);
       console.log("no match in: " + text);
     }
-  } else {
-    console.log("no match in " + node.textContent);
   }
+  return null;
 }
 
diff --git a/chrome/common/extensions/docs/examples/extensions/mappy/marker.png b/chrome/common/extensions/docs/examples/extensions/mappy/marker.png
index aa45f4d..3aa768b 100755
--- a/chrome/common/extensions/docs/examples/extensions/mappy/marker.png
+++ b/chrome/common/extensions/docs/examples/extensions/mappy/marker.png
diff --git a/chrome/common/extensions/docs/examples/extensions/mappy/popup.html b/chrome/common/extensions/docs/examples/extensions/mappy/popup.html
index 409b216..29d838d 100644
--- a/chrome/common/extensions/docs/examples/extensions/mappy/popup.html
+++ b/chrome/common/extensions/docs/examples/extensions/mappy/popup.html
@@ -31,18 +31,9 @@ function gclient_geocode(address) {
 }
 
 function map() {
-  chrome.tabs.getSelected(null, function(tab) {
-    var port = chrome.tabs.connect(tab.id);
-    if (!port) {
-      console.log("no port");
-    } else {
-      port.onMessage.addListener(function(data) {
-        var address = data.values[0];
-        gclient_geocode(address);
-      });
-      port.postMessage({message: "hello tab: " + tab.id});
-    }
-  });
+  var address = chrome.extension.getBackgroundPage().selectedAddress;
+  if (address)
+    gclient_geocode(address);
 };
 </script>
 </head>