Added populate parameter to chrome.windows.get, chrome.windows.getCurrent,

chrome/browser/extensions/extension_tabs_test.cc

Index: chrome/browser/extensions/extension_tabs_test.cc
===================================================================
--- chrome/browser/extensions/extension_tabs_test.cc	(revision 114830)
+++ chrome/browser/extensions/extension_tabs_test.cc	(working copy)
@@ -23,12 +23,12 @@
 #include "ui/gfx/rect.h"
 
 using namespace extension_function_test_utils;
+namespace keys = extension_tabs_module_constants;
 
 namespace {
 
 class ExtensionTabsTest : public InProcessBrowserTest {
 };
-
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionTabsTest, GetWindow) {
@@ -40,7 +40,7 @@
           new GetWindowFunction(),
           base::StringPrintf("[%u]", window_id + 1),
           browser()),
-      extension_tabs_module_constants::kWindowNotFoundError));
+      keys::kWindowNotFoundError));
 
   // Basic window details.
   // Need GetRestoredBound instead of GetBounds.
@@ -60,6 +60,18 @@
   EXPECT_EQ(bounds.width(), GetInteger(result.get(), "width"));
   EXPECT_EQ(bounds.height(), GetInteger(result.get(), "height"));
 
+  // With populate enabled
+  result.reset(ToDictionary(
+      RunFunctionAndReturnResult(
+          new GetWindowFunction(),
+          base::StringPrintf("[%u, {\"populate\": true}]", window_id),
+          browser())));
+
+  EXPECT_EQ(window_id, GetInteger(result.get(), "id"));
+  // populate was enabled so tabs should be populated
+  ListValue* tabs;

[Aaron Boodman, 2011/12/17 07:12:58]

*twitch*

+  EXPECT_TRUE(result.get()->GetList(keys::kTabsKey, &tabs));
+
   // TODO(aa): Can't assume window is focused. On mac, calling Activate() from a
   // browser test doesn't seem to do anything, so can't test the opposite
   // either.
@@ -101,7 +113,7 @@
           new GetWindowFunction(),
           base::StringPrintf("[%u]", incognito_window_id),
           browser()),
-      extension_tabs_module_constants::kWindowNotFoundError));
+      keys::kWindowNotFoundError));
 
   // With "include_incognito".
   result.reset(ToDictionary(
@@ -113,6 +125,124 @@
   EXPECT_TRUE(GetBoolean(result.get(), "incognito"));
 }
 
+IN_PROC_BROWSER_TEST_F(ExtensionTabsTest, GetCurrentWindow) {
+  int window_id = ExtensionTabUtil::GetWindowId(browser());
+  Browser* new_browser = CreateBrowser(browser()->profile());
+  int new_id = ExtensionTabUtil::GetWindowId(new_browser);
+
+  // Get the current window using new_browser
+  scoped_ptr<base::DictionaryValue> result(ToDictionary(
+      RunFunctionAndReturnResult(
+          new GetCurrentWindowFunction(),
+          "[]",
+          new_browser)));
+
+  // id should match the window that was passed to RunFunction

[Aaron Boodman, 2011/12/17 07:12:58]

Comment doesn't make sense. You mean something like: returned id should match
new_browser's id. Same comment multiple places in this file.

[Aaron Boodman, 2011/12/17 07:12:58]

Also, nit, but comments should generally be complete sentences, with
capitalization and periods (except in the case of end-of-line comments that
document a parameter).

[Matt Tytel, 2011/12/17 07:43:32]

Here I want to distinguish between the functionality of GetLastFocusedWindow.
That it should match whatever browser we "called" the GetCurrentWindowFunction
from.

[Aaron Boodman, 2011/12/17 08:02:40]

Ok, fair enough. In that case, it might be a little more clear to say something
like "...should match the window_id of the Browser instance that was passed to
RunFunction"  - to tie things back to the Browser class and new_browser
variable.

+  EXPECT_EQ(new_id, GetInteger(result.get(), "id"));
+  ListValue* tabs;

[Aaron Boodman, 2011/12/17 07:12:58]

!

+  EXPECT_FALSE(result.get()->GetList(keys::kTabsKey, &tabs));
+
+  // Get the current window using the old window and make the tabs populated
+  result.reset(ToDictionary(
+      RunFunctionAndReturnResult(
+          new GetCurrentWindowFunction(),
+          "[{\"populate\": true}]",
+          browser())));
+
+  // id should match the window that was passed to RunFunction
+  EXPECT_EQ(window_id, GetInteger(result.get(), "id"));
+  // populate was enabled so tabs should be populated
+  EXPECT_TRUE(result.get()->GetList(keys::kTabsKey, &tabs));
+}
+
+IN_PROC_BROWSER_TEST_F(ExtensionTabsTest, GetLastFocusedWindow) {
+  // Create a new window which making it the "last focused" window.
+  // Note that "last focused" means the "top" most window

[Aaron Boodman, 2011/12/17 07:12:58]

Missing final period.

+  Browser* new_browser = CreateBrowser(browser()->profile());
+  int focus_id = ExtensionTabUtil::GetWindowId(new_browser);

[Aaron Boodman, 2011/12/17 07:12:58]

focused_window_id?

+
+  scoped_ptr<base::DictionaryValue> result(ToDictionary(
+      RunFunctionAndReturnResult(
+          new GetLastFocusedWindowFunction(),
+          "[]",
+          new_browser)));
+
+  // id should match the last focused window
+  EXPECT_EQ(focus_id, GetInteger(result.get(), "id"));
+  ListValue* tabs;

[Aaron Boodman, 2011/12/17 07:12:58]

!! (I'm going to stop pointing these out now)

+  EXPECT_FALSE(result.get()->GetList(keys::kTabsKey, &tabs));
+
+  result.reset(ToDictionary(
+      RunFunctionAndReturnResult(
+          new GetLastFocusedWindowFunction(),
+          "[{\"populate\": true}]",
+          browser())));
+
+  // id should match the last focused window, not the window it was called from
+  EXPECT_EQ(focus_id, GetInteger(result.get(), "id"));
+  // populate was enabled so tabs should be populated
+  EXPECT_TRUE(result.get()->GetList(keys::kTabsKey, &tabs));
+}
+
+IN_PROC_BROWSER_TEST_F(ExtensionTabsTest, GetAllWindows) {
+  const size_t NUM_WINDOWS = 5;
+  std::vector<int> window_ids;
+  std::vector<int> result_ids;
+  window_ids.push_back(ExtensionTabUtil::GetWindowId(browser()));
+
+  for (uint i = 0; i < NUM_WINDOWS - 1; ++i) {
+    Browser* bro = CreateBrowser(browser()->profile());

[Aaron Boodman, 2011/12/17 07:12:58]

Avoid abbreviations. Just use the term "new_browser" here.

[Matt Tytel, 2011/12/17 07:43:32]

You got it bro ;)

+    window_ids.push_back(ExtensionTabUtil::GetWindowId(bro));
+  }
+  std::sort(window_ids.begin(), window_ids.end());
+
+  scoped_ptr<base::ListValue> result(ToList(
+      RunFunctionAndReturnResult(
+          new GetAllWindowsFunction(),
+          "[]",
+          browser())));
+
+  ListValue* windows = result.get();
+  EXPECT_EQ(NUM_WINDOWS, windows->GetSize());
+  for (uint i = 0; i < NUM_WINDOWS; ++i) {
+    DictionaryValue* result_window;
+    EXPECT_TRUE(windows->GetDictionary(i, &result_window));
+    result_ids.push_back(GetInteger(result_window, "id"));
+
+    // populate was not passed in so tabs are not populated
+    ListValue* tabs;
+    EXPECT_FALSE(result_window->GetList(keys::kTabsKey, &tabs));
+  }
+
+  std::sort(window_ids.begin(), window_ids.end());

[Aaron Boodman, 2011/12/17 07:12:58]

You should just use std::set. They come with an equality operator, so you don't
need to do this by hand. And you also don't need to sort them.

+  std::sort(result_ids.begin(), result_ids.end());
+  for (uint i = 0; i < NUM_WINDOWS; ++i)
+    EXPECT_EQ(window_ids[i], result_ids[i]);
+
+  result_ids.clear();
+  result.reset(ToList(
+      RunFunctionAndReturnResult(
+          new GetAllWindowsFunction(),
+          "[{\"populate\": true}]",
+          browser())));
+
+  windows = result.get();
+  EXPECT_EQ(NUM_WINDOWS, windows->GetSize());
+  for (uint i = 0; i < windows->GetSize(); ++i) {
+    DictionaryValue* result_window;
+    EXPECT_TRUE(windows->GetDictionary(i, &result_window));
+    result_ids.push_back(GetInteger(result_window, "id"));
+
+    // populate was enabled so tabs should be populated
+    ListValue* tabs;
+    EXPECT_TRUE(result_window->GetList(keys::kTabsKey, &tabs));
+  }
+
+  std::sort(result_ids.begin(), result_ids.end());
+  for (uint i = 0; i < NUM_WINDOWS; ++i)
+    EXPECT_EQ(window_ids[i], result_ids[i]);
+}
+
 IN_PROC_BROWSER_TEST_F(ExtensionTabsTest, UpdateNoPermissions) {
   // The test empty extension has no permissions, therefore it should not get
   // tab data in the function result.
@@ -216,7 +346,7 @@
           new CreateWindowFunction(),
           kArgsWithExplicitIncognitoParam,
           browser()),
-      extension_tabs_module_constants::kIncognitoModeIsForced));
+      keys::kIncognitoModeIsForced));
 
   // Now try opening a normal window from incognito window.
   Browser* incognito_browser = CreateIncognitoBrowser();
@@ -226,7 +356,7 @@
           new CreateWindowFunction(),
           kArgsWithExplicitIncognitoParam,
           incognito_browser),
-      extension_tabs_module_constants::kIncognitoModeIsForced));
+      keys::kIncognitoModeIsForced));
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionTabsTest,
@@ -244,7 +374,7 @@
           new CreateWindowFunction(),
           kArgs,
           browser()),
-      extension_tabs_module_constants::kIncognitoModeIsDisabled));
+      keys::kIncognitoModeIsDisabled));
 
   // Run in incognito window.
   EXPECT_TRUE(MatchPattern(
@@ -252,7 +382,7 @@
           new CreateWindowFunction(),
           kArgs,
           incognito_browser),
-      extension_tabs_module_constants::kIncognitoModeIsDisabled));
+      keys::kIncognitoModeIsDisabled));
 }
 
 #if defined(USE_AURA)
@@ -278,26 +408,26 @@
           new UpdateWindowFunction(),
           base::StringPrintf(kArgsMinimizedWithFocus, window_id),
           browser()),
-      extension_tabs_module_constants::kInvalidWindowStateError));
+      keys::kInvalidWindowStateError));
 
   EXPECT_TRUE(MatchPattern(
       RunFunctionAndReturnError(
           new UpdateWindowFunction(),
           base::StringPrintf(kArgsMaximizedWithoutFocus, window_id),
           browser()),
-      extension_tabs_module_constants::kInvalidWindowStateError));
+      keys::kInvalidWindowStateError));
 
   EXPECT_TRUE(MatchPattern(
       RunFunctionAndReturnError(
           new UpdateWindowFunction(),
           base::StringPrintf(kArgsMinimizedWithBounds, window_id),
           browser()),
-      extension_tabs_module_constants::kInvalidWindowStateError));
+      keys::kInvalidWindowStateError));
 
   EXPECT_TRUE(MatchPattern(
       RunFunctionAndReturnError(
           new UpdateWindowFunction(),
           base::StringPrintf(kArgsMaximizedWithBounds, window_id),
           browser()),
-      extension_tabs_module_constants::kInvalidWindowStateError));
+      keys::kInvalidWindowStateError));
 }