Don't call HandleGetMostVisited on the backend after each black list

operation. Instead let JS do the call as needed. This is in preparation
for the new new tab page which will do other things when an URL is
blacklisted.

Also, fix some issue in the last CL

BUG=13362

TEST=Add and clear most visted items like before. Things should work as
before

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

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

2 files changed, 25 insertions, 22 deletions

diff --git a/chrome/browser/dom_ui/new_tab_ui.cc b/chrome/browser/dom_ui/new_tab_ui.cc
index 54731a2..451369d 100644
--- a/chrome/browser/dom_ui/new_tab_ui.cc
+++ b/chrome/browser/dom_ui/new_tab_ui.cc
@@ -563,8 +563,6 @@ void MostVisitedHandler::HandleBlacklistURL(const Value* value) {
     return;
   }
   BlacklistURL(GURL(url));
-  // Force a refresh of the thumbnails.
-  HandleGetMostVisited(NULL);
 }
 
 void MostVisitedHandler::HandleRemoveURLsFromBlacklist(const Value* urls) {
@@ -589,15 +587,10 @@ void MostVisitedHandler::HandleRemoveURLsFromBlacklist(const Value* urls) {
     r = url_blacklist_->Remove(GetDictionaryKeyForURL(WideToUTF8(url)), NULL);
     DCHECK(r) << "Unknown URL removed from the NTP Most Visited blacklist.";
   }
-
-  // Force a refresh of the thumbnails.
-  HandleGetMostVisited(NULL);
 }
 
 void MostVisitedHandler::HandleClearBlacklist(const Value* value) {
   url_blacklist_->Clear();
-  // Force a refresh of the thumbnails.
-  HandleGetMostVisited(NULL);
 }
 
 void MostVisitedHandler::HandleAddPinnedURL(const Value* value) {
@@ -692,7 +685,7 @@ const bool MostVisitedHandler::GetPinnedURLAtIndex(const int index,
       DictionaryValue* dict = static_cast<DictionaryValue*>(value);
       dict->GetInteger(L"index", &dict_index);
       if (dict_index == index) {
-        if (dict->GetString(L"url", url))
+        if (!dict->GetString(L"url", url))
           return false;
         return dict->GetString(L"title", title);
       }
@@ -710,24 +703,23 @@ void MostVisitedHandler::OnSegmentUsageAvailable(
   most_visited_urls_.clear();
   ListValue pages_value;
 
-  size_t i = 0;
-  size_t j = 0;
-  while (j < kMostVisitedPages && i < data->size()) {
+  size_t data_index = 0;
+  size_t output_index = 0;
+  while (output_index < kMostVisitedPages && data_index < data->size()) {
     bool pinned = false;
     GURL url;
     string16 title;
     std::string pinned_url;
     std::string pinned_title;
 
-    if (MostVisitedHandler::GetPinnedURLAtIndex(j, &pinned_url,
+    if (MostVisitedHandler::GetPinnedURLAtIndex(output_index, &pinned_url,
                                                 &pinned_title)) {
       url = GURL(pinned_url);
       title = UTF8ToUTF16(pinned_title);
       pinned = true;
-      j++;
     } else {
-      const PageUsageData& page = *(*data)[i];
-      i++;
+      const PageUsageData& page = *(*data)[data_index];
+      data_index++;
       url = page.GetURL();
 
       // Don't include blacklisted or pinned URLs.
@@ -743,6 +735,7 @@ void MostVisitedHandler::OnSegmentUsageAvailable(
     SetURLTitleAndDirection(page_value, title, url);
     page_value->SetBoolean(L"pinned", pinned);
     pages_value.Append(page_value);
+    output_index++;
     most_visited_urls_.push_back(url);
   }
 
diff --git a/chrome/browser/resources/new_tab.html b/chrome/browser/resources/new_tab.html
index ba582fe..2bd5389 100644
--- a/chrome/browser/resources/new_tab.html
+++ b/chrome/browser/resources/new_tab.html
@@ -102,10 +102,10 @@ function resizeP13N(new_height) {
    childf.style.display = "block";
 }
 
-chrome.send("getMostVisited");
-chrome.send("getMostSearched");
-chrome.send("getRecentlyBookmarked");
-chrome.send("getRecentlyClosedTabs");
+chrome.send('getMostVisited');
+chrome.send('getMostSearched');
+chrome.send('getRecentlyBookmarked');
+chrome.send('getRecentlyClosedTabs');
 
 function handleWindowResize() {
   if (!document.body || document.body.clientWidth < 10) {
@@ -926,19 +926,29 @@ function exitEditMode() {
 }
 
 function cancelEdits() {
-  if (blacklistedURLs.length > 0)
-    chrome.send("removeURLsFromMostVisitedBlacklist", blacklistedURLs);
+  if (blacklistedURLs.length > 0) {
+    chrome.send('removeURLsFromMostVisitedBlacklist', blacklistedURLs);
+    // NOTE(arv): According to evanm the order of messages should be the order
+    // they are called in. However, he wasn't sure and glen wasn't sure either.
+    // We should keep our eyes open for weird issues and if the order is not
+    // guaranteed we can call 'getMostVisited' in a timeout.
+    chrome.send('getMostVisited');
+  }
   exitEditMode();
 }
 
 function blacklistURL(url) {
  blacklistedURLs.push(url);
- chrome.send("blacklistURLFromMostVisited", [url]);
+ chrome.send('blacklistURLFromMostVisited', [url]);
+ // NOTE(arv): See note in cancelEdits.
+ chrome.send('getMostVisited');
 }
 
 function restoreThumbnails() {
   exitEditMode();
   chrome.send('clearMostVisitedURLsBlacklist');
+  // NOTE(arv): See note in cancelEdits.
+  chrome.send('getMostVisited');
 }
 
 function themeChanged() {