[Extensions] Move some tabs API-related functionally out of WindowController
Move WindowController::CreateWindowValue(),
WindowController::CreateWindowValueWithTabs(), and
WindowController::CreateTabObject() to ExtensionTabUtil, and make the
window value creation methods take a Browser object instead of a
WindowController. Since the Windows API should not have knowledge of
AppWindows, this helps us elminate unnecessary complexity from the
WindowController class (which is used for both normal browser and app
windows), as well as helping to ensure that we don't expose knowledge
we shouldn't.
Bug: 805528
Change-Id: Ia8dd2a636d552d4bb70768ab2a3019c537fdb5ab
Reviewed-on: https://chromium-review.googlesource.com/902322
Commit-Queue: Devlin <[email protected]>
Reviewed-by: Karan Bhatia <[email protected]>
Cr-Commit-Position: refs/heads/master@{#535107}diff --git a/chrome/browser/extensions/api/sessions/sessions_api.cc b/chrome/browser/extensions/api/sessions/sessions_api.cc
index aba5cc99..a575a4c 100644
--- a/chrome/browser/extensions/api/sessions/sessions_api.cc
+++ b/chrome/browser/extensions/api/sessions/sessions_api.cc
@@ -6,6 +6,7 @@
#include <stddef.h>
+#include <algorithm>
#include <memory>
#include <utility>
#include <vector>
@@ -390,14 +391,15 @@
ExtensionFunction::ResponseValue
SessionsRestoreFunction::GetRestoredWindowResult(int window_id) {
- WindowController* controller = NULL;
+ Browser* browser = nullptr;
std::string error;
- if (!windows_util::GetWindowFromWindowID(this, window_id, 0, &controller,
- &error)) {
+ if (!windows_util::GetBrowserFromWindowID(this, window_id, 0, &browser,
+ &error)) {
return Error(error);
}
std::unique_ptr<base::DictionaryValue> window_value(
- controller->CreateWindowValueWithTabs(extension()));
+ ExtensionTabUtil::CreateWindowValueForExtension(
+ *browser, extension(), ExtensionTabUtil::kPopulateTabs));
std::unique_ptr<windows::Window> window(
windows::Window::FromValue(*window_value));
return ArgumentList(Restore::Results::Create(*CreateSessionModelHelper(
diff --git a/chrome/browser/extensions/api/tabs/app_window_controller.cc b/chrome/browser/extensions/api/tabs/app_window_controller.cc
index fb9f4f1..458fc5b 100644
--- a/chrome/browser/extensions/api/tabs/app_window_controller.cc
+++ b/chrome/browser/extensions/api/tabs/app_window_controller.cc
@@ -7,14 +7,11 @@
#include <memory>
#include <utility>
-#include "base/strings/utf_string_conversions.h"
-#include "base/values.h"
#include "chrome/browser/extensions/api/tabs/app_base_window.h"
#include "chrome/browser/extensions/api/tabs/tabs_constants.h"
#include "chrome/browser/extensions/extension_tab_util.h"
#include "chrome/browser/extensions/window_controller.h"
#include "chrome/browser/extensions/window_controller_list.h"
-#include "chrome/browser/sessions/session_tab_helper.h"
#include "chrome/common/url_constants.h"
#include "extensions/browser/app_window/app_window.h"
#include "extensions/browser/app_window/native_app_window.h"
@@ -46,62 +43,6 @@
return tabs_constants::kWindowTypeValueApp;
}
-std::unique_ptr<base::DictionaryValue>
-AppWindowController::CreateWindowValueWithTabs(
- const Extension* extension) const {
- std::unique_ptr<base::DictionaryValue> result = CreateWindowValue();
-
- std::unique_ptr<base::DictionaryValue> tab_value =
- CreateTabObject(extension, 0)->ToValue();
- if (!tab_value)
- return result;
-
- auto tab_list = std::make_unique<base::ListValue>();
- tab_list->Append(std::move(tab_value));
- result->Set(tabs_constants::kTabsKey, std::move(tab_list));
-
- return result;
-}
-
-std::unique_ptr<api::tabs::Tab> AppWindowController::CreateTabObject(
- const extensions::Extension* extension,
- int tab_index) const {
- if (tab_index > 0)
- return nullptr;
-
- content::WebContents* web_contents = app_window_->web_contents();
- if (!web_contents)
- return nullptr;
-
- std::unique_ptr<api::tabs::Tab> tab_object(new api::tabs::Tab);
- tab_object->id.reset(new int(SessionTabHelper::IdForTab(web_contents)));
- tab_object->index = 0;
- tab_object->window_id =
- SessionTabHelper::IdForWindowContainingTab(web_contents);
- tab_object->url.reset(new std::string(web_contents->GetURL().spec()));
- tab_object->status.reset(new std::string(
- ExtensionTabUtil::GetTabStatusText(web_contents->IsLoading())));
- tab_object->active = app_window_->GetBaseWindow()->IsActive();
- tab_object->selected = true;
- tab_object->highlighted = true;
- tab_object->pinned = false;
- tab_object->title.reset(
- new std::string(base::UTF16ToUTF8(web_contents->GetTitle())));
- tab_object->incognito = app_window_->GetBaseWindow()->IsActive();
- gfx::Rect bounds = app_window_->GetBaseWindow()->GetBounds();
- tab_object->width.reset(new int(bounds.width()));
- tab_object->height.reset(new int(bounds.height()));
-
- const Extension* ext = app_window_->GetExtension();
- if (ext) {
- std::string icon_str(chrome::kChromeUIFaviconURL);
- icon_str.append(app_window_->GetExtension()->url().spec());
- tab_object->fav_icon_url.reset(new std::string(icon_str));
- }
-
- return tab_object;
-}
-
bool AppWindowController::CanClose(Reason* reason) const {
return true;
}
diff --git a/chrome/browser/extensions/api/tabs/app_window_controller.h b/chrome/browser/extensions/api/tabs/app_window_controller.h
index 848739f..a5fd585 100644
--- a/chrome/browser/extensions/api/tabs/app_window_controller.h
+++ b/chrome/browser/extensions/api/tabs/app_window_controller.h
@@ -5,6 +5,7 @@
#ifndef CHROME_BROWSER_EXTENSIONS_API_TABS_APP_WINDOW_CONTROLLER_H_
#define CHROME_BROWSER_EXTENSIONS_API_TABS_APP_WINDOW_CONTROLLER_H_
+#include <memory>
#include <string>
#include "base/macros.h"
@@ -28,12 +29,6 @@
// extensions::WindowController:
int GetWindowId() const override;
std::string GetWindowTypeText() const override;
- std::unique_ptr<base::DictionaryValue> CreateWindowValueWithTabs(
- const Extension* extension) const override;
- std::unique_ptr<api::tabs::Tab> CreateTabObject(
- const extensions::Extension* extension,
- int tab_index) const override;
-
bool CanClose(Reason* reason) const override;
void SetFullscreenMode(bool is_fullscreen,
const GURL& extension_url) const override;
diff --git a/chrome/browser/extensions/api/tabs/tabs_api.cc b/chrome/browser/extensions/api/tabs/tabs_api.cc
index bfdb046..8f6f6c7 100644
--- a/chrome/browser/extensions/api/tabs/tabs_api.cc
+++ b/chrome/browser/extensions/api/tabs/tabs_api.cc
@@ -304,18 +304,20 @@
EXTENSION_FUNCTION_VALIDATE(params.get());
ApiParameterExtractor<windows::Get::Params> extractor(params.get());
- WindowController* controller = nullptr;
+ Browser* browser = nullptr;
std::string error;
- if (!windows_util::GetWindowFromWindowID(this, params->window_id,
- extractor.type_filters(),
- &controller, &error)) {
+ if (!windows_util::GetBrowserFromWindowID(this, params->window_id,
+ extractor.type_filters(), &browser,
+ &error)) {
return RespondNow(Error(error));
}
+ ExtensionTabUtil::PopulateTabBehavior populate_tab_behavior =
+ extractor.populate_tabs() ? ExtensionTabUtil::kPopulateTabs
+ : ExtensionTabUtil::kDontPopulateTabs;
std::unique_ptr<base::DictionaryValue> windows =
- extractor.populate_tabs()
- ? controller->CreateWindowValueWithTabs(extension())
- : controller->CreateWindowValue();
+ ExtensionTabUtil::CreateWindowValueForExtension(*browser, extension(),
+ populate_tab_behavior);
return RespondNow(OneArgument(std::move(windows)));
}
@@ -325,18 +327,20 @@
EXTENSION_FUNCTION_VALIDATE(params.get());
ApiParameterExtractor<windows::GetCurrent::Params> extractor(params.get());
- WindowController* controller = nullptr;
+ Browser* browser = nullptr;
std::string error;
- if (!windows_util::GetWindowFromWindowID(
+ if (!windows_util::GetBrowserFromWindowID(
this, extension_misc::kCurrentWindowId, extractor.type_filters(),
- &controller, &error)) {
+ &browser, &error)) {
return RespondNow(Error(error));
}
+ ExtensionTabUtil::PopulateTabBehavior populate_tab_behavior =
+ extractor.populate_tabs() ? ExtensionTabUtil::kPopulateTabs
+ : ExtensionTabUtil::kDontPopulateTabs;
std::unique_ptr<base::DictionaryValue> windows =
- extractor.populate_tabs()
- ? controller->CreateWindowValueWithTabs(extension())
- : controller->CreateWindowValue();
+ ExtensionTabUtil::CreateWindowValueForExtension(*browser, extension(),
+ populate_tab_behavior);
return RespondNow(OneArgument(std::move(windows)));
}
@@ -349,22 +353,28 @@
params.get());
// The WindowControllerList should contain a list of application,
// browser and devtools windows.
- WindowController* controller = nullptr;
- for (auto* iter : WindowControllerList::GetInstance()->windows()) {
- if (windows_util::CanOperateOnWindow(this, iter,
+ Browser* browser = nullptr;
+ for (auto* controller : WindowControllerList::GetInstance()->windows()) {
+ if (controller->GetBrowser() &&
+ windows_util::CanOperateOnWindow(this, controller,
extractor.type_filters())) {
- controller = iter;
+ // TODO(devlin): Doesn't this mean that we'll use the last window in the
+ // list if there is no active window? That seems wrong.
+ // See https://crbug.com/809822.
+ browser = controller->GetBrowser();
if (controller->window()->IsActive())
break; // Use focused window.
}
}
- if (!controller)
+ if (!browser)
return RespondNow(Error(keys::kNoLastFocusedWindowError));
+ ExtensionTabUtil::PopulateTabBehavior populate_tab_behavior =
+ extractor.populate_tabs() ? ExtensionTabUtil::kPopulateTabs
+ : ExtensionTabUtil::kDontPopulateTabs;
std::unique_ptr<base::DictionaryValue> windows =
- extractor.populate_tabs()
- ? controller->CreateWindowValueWithTabs(extension())
- : controller->CreateWindowValue();
+ ExtensionTabUtil::CreateWindowValueForExtension(*browser, extension(),
+ populate_tab_behavior);
return RespondNow(OneArgument(std::move(windows)));
}
@@ -375,18 +385,17 @@
ApiParameterExtractor<windows::GetAll::Params> extractor(params.get());
std::unique_ptr<base::ListValue> window_list(new base::ListValue());
- const WindowControllerList::ControllerList& windows =
- WindowControllerList::GetInstance()->windows();
- for (WindowControllerList::ControllerList::const_iterator iter =
- windows.begin();
- iter != windows.end(); ++iter) {
- if (!windows_util::CanOperateOnWindow(this, *iter,
- extractor.type_filters()))
+ ExtensionTabUtil::PopulateTabBehavior populate_tab_behavior =
+ extractor.populate_tabs() ? ExtensionTabUtil::kPopulateTabs
+ : ExtensionTabUtil::kDontPopulateTabs;
+ for (auto* controller : WindowControllerList::GetInstance()->windows()) {
+ if (!controller->GetBrowser() ||
+ !windows_util::CanOperateOnWindow(this, controller,
+ extractor.type_filters())) {
continue;
- if (extractor.populate_tabs())
- window_list->Append((*iter)->CreateWindowValueWithTabs(extension()));
- else
- window_list->Append((*iter)->CreateWindowValue());
+ }
+ window_list->Append(ExtensionTabUtil::CreateWindowValueForExtension(
+ *controller->GetBrowser(), extension(), populate_tab_behavior));
}
return RespondNow(OneArgument(std::move(window_list)));
@@ -649,8 +658,6 @@
else
new_window->window()->ShowInactive();
- WindowController* controller = new_window->extension_window_controller();
-
std::unique_ptr<base::Value> result;
if (new_window->profile()->IsOffTheRecord() &&
!browser_context()->IsOffTheRecord() && !include_incognito()) {
@@ -658,7 +665,8 @@
// profile and CanCrossIncognito isn't allowed.
result = std::make_unique<base::Value>();
} else {
- result = controller->CreateWindowValueWithTabs(extension());
+ result = ExtensionTabUtil::CreateWindowValueForExtension(
+ *new_window, extension(), ExtensionTabUtil::kPopulateTabs);
}
return RespondNow(OneArgument(std::move(result)));
@@ -669,11 +677,11 @@
windows::Update::Params::Create(*args_));
EXTENSION_FUNCTION_VALIDATE(params);
- WindowController* controller;
+ Browser* browser = nullptr;
std::string error;
- if (!windows_util::GetWindowFromWindowID(
+ if (!windows_util::GetBrowserFromWindowID(
this, params->window_id, WindowController::GetAllWindowFilter(),
- &controller, &error)) {
+ &browser, &error)) {
return RespondNow(Error(error));
}
@@ -683,7 +691,7 @@
#if defined(OS_CHROMEOS)
const bool is_window_trusted_pinned =
- ash::IsWindowTrustedPinned(controller->window());
+ ash::IsWindowTrustedPinned(browser->window());
// Don't allow locked fullscreen operations on a window without the proper
// permission (also don't allow any operations on a locked window if the
// extension doesn't have the permission).
@@ -698,15 +706,13 @@
if (is_window_trusted_pinned &&
params->update_info.state != windows::WINDOW_STATE_LOCKED_FULLSCREEN &&
params->update_info.state != windows::WINDOW_STATE_NONE) {
- SetWindowTrustedPinned(controller->window(), false);
- controller->GetBrowser()->command_controller()->
- LockedFullscreenStateChanged();
+ SetWindowTrustedPinned(browser->window(), false);
+ browser->command_controller()->LockedFullscreenStateChanged();
} else if (!is_window_trusted_pinned &&
params->update_info.state ==
windows::WINDOW_STATE_LOCKED_FULLSCREEN) {
- SetWindowTrustedPinned(controller->window(), true);
- controller->GetBrowser()->command_controller()->
- LockedFullscreenStateChanged();
+ SetWindowTrustedPinned(browser->window(), true);
+ browser->command_controller()->LockedFullscreenStateChanged();
}
#endif
@@ -714,34 +720,38 @@
ConvertToWindowShowState(params->update_info.state);
if (show_state != ui::SHOW_STATE_FULLSCREEN &&
- show_state != ui::SHOW_STATE_DEFAULT)
- controller->SetFullscreenMode(false, extension()->url());
+ show_state != ui::SHOW_STATE_DEFAULT) {
+ browser->extension_window_controller()->SetFullscreenMode(
+ false, extension()->url());
+ }
switch (show_state) {
case ui::SHOW_STATE_MINIMIZED:
- controller->window()->Minimize();
+ browser->window()->Minimize();
break;
case ui::SHOW_STATE_MAXIMIZED:
- controller->window()->Maximize();
+ browser->window()->Maximize();
break;
case ui::SHOW_STATE_FULLSCREEN:
- if (controller->window()->IsMinimized() ||
- controller->window()->IsMaximized())
- controller->window()->Restore();
- controller->SetFullscreenMode(true, extension()->url());
+ if (browser->window()->IsMinimized() ||
+ browser->window()->IsMaximized()) {
+ browser->window()->Restore();
+ }
+ browser->extension_window_controller()->SetFullscreenMode(
+ true, extension()->url());
break;
case ui::SHOW_STATE_NORMAL:
- controller->window()->Restore();
+ browser->window()->Restore();
break;
default:
break;
}
gfx::Rect bounds;
- if (controller->window()->IsMinimized())
- bounds = controller->window()->GetRestoredBounds();
+ if (browser->window()->IsMinimized())
+ bounds = browser->window()->GetRestoredBounds();
else
- bounds = controller->window()->GetBounds();
+ bounds = browser->window()->GetBounds();
bool set_bounds = false;
// Any part of the bounds can optionally be set by the caller.
@@ -773,27 +783,28 @@
}
// TODO(varkha): Updating bounds during a drag can cause problems and a more
// general solution is needed. See http://crbug.com/251813 .
- controller->window()->SetBounds(bounds);
+ browser->window()->SetBounds(bounds);
}
if (params->update_info.focused) {
if (*params->update_info.focused) {
if (show_state == ui::SHOW_STATE_MINIMIZED)
return RespondNow(Error(keys::kInvalidWindowStateError));
- controller->window()->Activate();
+ browser->window()->Activate();
} else {
if (show_state == ui::SHOW_STATE_MAXIMIZED ||
show_state == ui::SHOW_STATE_FULLSCREEN) {
return RespondNow(Error(keys::kInvalidWindowStateError));
}
- controller->window()->Deactivate();
+ browser->window()->Deactivate();
}
}
if (params->update_info.draw_attention)
- controller->window()->FlashFrame(*params->update_info.draw_attention);
+ browser->window()->FlashFrame(*params->update_info.draw_attention);
- return RespondNow(OneArgument(controller->CreateWindowValue()));
+ return RespondNow(OneArgument(ExtensionTabUtil::CreateWindowValueForExtension(
+ *browser, extension(), ExtensionTabUtil::kDontPopulateTabs)));
}
ExtensionFunction::ResponseAction WindowsRemoveFunction::Run() {
@@ -801,22 +812,23 @@
windows::Remove::Params::Create(*args_));
EXTENSION_FUNCTION_VALIDATE(params);
- WindowController* controller = nullptr;
+ Browser* browser = nullptr;
std::string error;
- if (!windows_util::GetWindowFromWindowID(this, params->window_id,
- WindowController::kNoWindowFilter,
- &controller, &error)) {
+ if (!windows_util::GetBrowserFromWindowID(this, params->window_id,
+ WindowController::kNoWindowFilter,
+ &browser, &error)) {
return RespondNow(Error(error));
}
#if defined(OS_CHROMEOS)
- if (ash::IsWindowTrustedPinned(controller->window()) &&
+ if (ash::IsWindowTrustedPinned(browser->window()) &&
!ExtensionHasLockedFullscreenPermission(extension())) {
return RespondNow(
Error(keys::kMissingLockWindowFullscreenPrivatePermission));
}
#endif
+ WindowController* controller = browser->extension_window_controller();
WindowController::Reason reason;
if (!controller->CanClose(&reason)) {
return RespondNow(Error(reason == WindowController::REASON_NOT_EDITABLE
@@ -1179,9 +1191,8 @@
selection.set_active(active_index);
browser->tab_strip_model()->SetSelectionFromModel(std::move(selection));
- return RespondNow(OneArgument(
- browser->extension_window_controller()->CreateWindowValueWithTabs(
- extension())));
+ return RespondNow(OneArgument(ExtensionTabUtil::CreateWindowValueForExtension(
+ *browser, extension(), ExtensionTabUtil::kPopulateTabs)));
}
bool TabsHighlightFunction::HighlightTab(TabStripModel* tabstrip,
diff --git a/chrome/browser/extensions/api/tabs/windows_event_router.cc b/chrome/browser/extensions/api/tabs/windows_event_router.cc
index 98df5ae..7e44121 100644
--- a/chrome/browser/extensions/api/tabs/windows_event_router.cc
+++ b/chrome/browser/extensions/api/tabs/windows_event_router.cc
@@ -14,6 +14,7 @@
#include "chrome/browser/extensions/api/tabs/tabs_constants.h"
#include "chrome/browser/extensions/api/tabs/windows_util.h"
#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_tab_util.h"
#include "chrome/browser/extensions/extension_util.h"
#include "chrome/browser/extensions/window_controller.h"
#include "chrome/browser/extensions/window_controller_list.h"
@@ -205,9 +206,14 @@
return;
if (!profile_->IsSameProfile(window_controller->profile()))
return;
+ // Ignore any windows without an associated browser (e.g., AppWindows).
+ if (!window_controller->GetBrowser())
+ return;
std::unique_ptr<base::ListValue> args(new base::ListValue());
- args->Append(window_controller->CreateWindowValue());
+ args->Append(ExtensionTabUtil::CreateWindowValueForExtension(
+ *window_controller->GetBrowser(), nullptr,
+ ExtensionTabUtil::kDontPopulateTabs));
DispatchEvent(events::WINDOWS_ON_CREATED, windows::OnCreated::kEventName,
window_controller, std::move(args));
}
@@ -218,6 +224,9 @@
return;
if (!profile_->IsSameProfile(window_controller->profile()))
return;
+ // Ignore any windows without an associated browser (e.g., AppWindows).
+ if (!window_controller->GetBrowser())
+ return;
int window_id = window_controller->GetWindowId();
std::unique_ptr<base::ListValue> args(new base::ListValue());
diff --git a/chrome/browser/extensions/api/tabs/windows_util.cc b/chrome/browser/extensions/api/tabs/windows_util.cc
index 1b8a467..3d75225 100644
--- a/chrome/browser/extensions/api/tabs/windows_util.cc
+++ b/chrome/browser/extensions/api/tabs/windows_util.cc
@@ -23,38 +23,48 @@
namespace windows_util {
-bool GetWindowFromWindowID(UIThreadExtensionFunction* function,
- int window_id,
- extensions::WindowController::TypeFilter filter,
- extensions::WindowController** controller,
- std::string* error) {
+bool GetBrowserFromWindowID(UIThreadExtensionFunction* function,
+ int window_id,
+ extensions::WindowController::TypeFilter filter,
+ Browser** browser,
+ std::string* error) {
+ DCHECK(browser);
DCHECK(error);
+
+ *browser = nullptr;
if (window_id == extension_misc::kCurrentWindowId) {
- extensions::WindowController* extension_window_controller =
- function->dispatcher()->GetExtensionWindowController();
// If there is a window controller associated with this extension, use that.
- if (extension_window_controller) {
- *controller = extension_window_controller;
- } else {
+ extensions::WindowController* window_controller =
+ function->dispatcher()->GetExtensionWindowController();
+ if (!window_controller) {
// Otherwise get the focused or most recently added window.
- *controller = extensions::WindowControllerList::GetInstance()
- ->CurrentWindowForFunctionWithFilter(function, filter);
+ window_controller =
+ extensions::WindowControllerList::GetInstance()
+ ->CurrentWindowForFunctionWithFilter(function, filter);
}
- if (!(*controller)) {
+
+ if (window_controller)
+ *browser = window_controller->GetBrowser();
+
+ if (!(*browser)) {
*error = extensions::tabs_constants::kNoCurrentWindowError;
return false;
}
} else {
- *controller =
+ extensions::WindowController* window_controller =
extensions::WindowControllerList::GetInstance()
->FindWindowForFunctionByIdWithFilter(function, window_id, filter);
- if (!(*controller)) {
+ if (window_controller)
+ *browser = window_controller->GetBrowser();
+
+ if (!(*browser)) {
*error = extensions::ErrorUtils::FormatErrorMessage(
extensions::tabs_constants::kWindowNotFoundError,
base::IntToString(window_id));
return false;
}
}
+ DCHECK(*browser);
return true;
}
diff --git a/chrome/browser/extensions/api/tabs/windows_util.h b/chrome/browser/extensions/api/tabs/windows_util.h
index d5526e1..f080d06 100644
--- a/chrome/browser/extensions/api/tabs/windows_util.h
+++ b/chrome/browser/extensions/api/tabs/windows_util.h
@@ -5,6 +5,8 @@
#ifndef CHROME_BROWSER_EXTENSIONS_API_TABS_WINDOWS_UTIL_H__
#define CHROME_BROWSER_EXTENSIONS_API_TABS_WINDOWS_UTIL_H__
+#include <string>
+
#include "chrome/browser/extensions/window_controller_list.h"
class UIThreadExtensionFunction;
@@ -15,13 +17,13 @@
namespace windows_util {
-// Populates |controller| for given |window_id|. If the window is not found,
+// Populates |browser| for given |window_id|. If the window is not found,
// returns false and sets |error|.
-bool GetWindowFromWindowID(UIThreadExtensionFunction* function,
- int window_id,
- extensions::WindowController::TypeFilter filter,
- extensions::WindowController** controller,
- std::string* error);
+bool GetBrowserFromWindowID(UIThreadExtensionFunction* function,
+ int window_id,
+ extensions::WindowController::TypeFilter filter,
+ Browser** browser,
+ std::string* error);
// Returns true if |function| (and the profile and extension that it was
// invoked from) can operate on the window wrapped by |window_controller|.
diff --git a/chrome/browser/extensions/browser_extension_window_controller.cc b/chrome/browser/extensions/browser_extension_window_controller.cc
index e635289..e6fee62 100644
--- a/chrome/browser/extensions/browser_extension_window_controller.cc
+++ b/chrome/browser/extensions/browser_extension_window_controller.cc
@@ -4,6 +4,8 @@
#include "chrome/browser/extensions/browser_extension_window_controller.h"
+#include <string>
+
#include "chrome/browser/extensions/api/tabs/tabs_constants.h"
#include "chrome/browser/extensions/extension_tab_util.h"
#include "chrome/browser/extensions/window_controller_list.h"
@@ -32,33 +34,7 @@
namespace keys = extensions::tabs_constants;
std::string BrowserExtensionWindowController::GetWindowTypeText() const {
- if (browser_->is_devtools())
- return keys::kWindowTypeValueDevTools;
- if (browser_->is_type_popup())
- return keys::kWindowTypeValuePopup;
- if (browser_->is_app())
- return keys::kWindowTypeValueApp;
- return keys::kWindowTypeValueNormal;
-}
-
-std::unique_ptr<base::DictionaryValue>
-BrowserExtensionWindowController::CreateWindowValueWithTabs(
- const extensions::Extension* extension) const {
- std::unique_ptr<base::DictionaryValue> result = CreateWindowValue();
-
- result->Set(keys::kTabsKey,
- extensions::ExtensionTabUtil::CreateTabList(browser_, extension));
-
- return result;
-}
-
-std::unique_ptr<extensions::api::tabs::Tab>
-BrowserExtensionWindowController::CreateTabObject(
- const extensions::Extension* extension,
- int tab_index) const {
- TabStripModel* tab_strip = browser_->tab_strip_model();
- return extensions::ExtensionTabUtil::CreateTabObject(
- tab_strip->GetWebContentsAt(tab_index), tab_strip, tab_index);
+ return extensions::ExtensionTabUtil::GetBrowserWindowTypeText(*browser_);
}
bool BrowserExtensionWindowController::CanClose(Reason* reason) const {
diff --git a/chrome/browser/extensions/browser_extension_window_controller.h b/chrome/browser/extensions/browser_extension_window_controller.h
index 593f119d..15e7d9d3 100644
--- a/chrome/browser/extensions/browser_extension_window_controller.h
+++ b/chrome/browser/extensions/browser_extension_window_controller.h
@@ -22,11 +22,6 @@
// extensions::WindowController implementation.
int GetWindowId() const override;
std::string GetWindowTypeText() const override;
- std::unique_ptr<base::DictionaryValue> CreateWindowValueWithTabs(
- const extensions::Extension* extension) const override;
- std::unique_ptr<extensions::api::tabs::Tab> CreateTabObject(
- const extensions::Extension* extension,
- int tab_index) const override;
bool CanClose(Reason* reason) const override;
void SetFullscreenMode(bool is_fullscreen,
const GURL& extension_url) const override;
diff --git a/chrome/browser/extensions/extension_tab_util.cc b/chrome/browser/extensions/extension_tab_util.cc
index 96cc226..9ad44bfc 100644
--- a/chrome/browser/extensions/extension_tab_util.cc
+++ b/chrome/browser/extensions/extension_tab_util.cc
@@ -5,6 +5,7 @@
#include "chrome/browser/extensions/extension_tab_util.h"
#include <stddef.h>
+#include <algorithm>
#include "base/macros.h"
#include "base/strings/string_number_conversions.h"
@@ -15,8 +16,6 @@
#include "chrome/browser/extensions/chrome_extension_function.h"
#include "chrome/browser/extensions/chrome_extension_function_details.h"
#include "chrome/browser/extensions/tab_helper.h"
-#include "chrome/browser/extensions/window_controller.h"
-#include "chrome/browser/extensions/window_controller_list.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/resource_coordinator/tab_manager.h"
#include "chrome/browser/sessions/session_tab_helper.h"
@@ -35,8 +34,6 @@
#include "content/public/browser/favicon_status.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/web_contents.h"
-#include "extensions/browser/app_window/app_window.h"
-#include "extensions/browser/app_window/app_window_registry.h"
#include "extensions/common/constants.h"
#include "extensions/common/error_utils.h"
#include "extensions/common/extension.h"
@@ -48,6 +45,10 @@
#include "extensions/common/permissions/permissions_data.h"
#include "url/gurl.h"
+#if defined(OS_CHROMEOS)
+#include "ash/public/cpp/window_pin_type.h"
+#endif
+
using content::NavigationEntry;
using content::WebContents;
@@ -57,18 +58,6 @@
namespace keys = tabs_constants;
-WindowController* GetAppWindowController(const WebContents* contents) {
- Profile* profile = Profile::FromBrowserContext(contents->GetBrowserContext());
- AppWindowRegistry* registry = AppWindowRegistry::Get(profile);
- if (!registry)
- return nullptr;
- AppWindow* app_window = registry->GetAppWindowForWebContents(contents);
- if (!app_window)
- return nullptr;
- return WindowControllerList::GetInstance()->FindWindowById(
- app_window->session_id().id());
-}
-
// |error_message| can optionally be passed in and will be set with an
// appropriate message if the window cannot be found by id.
Browser* GetBrowserInProfileWithId(Profile* profile,
@@ -328,19 +317,26 @@
}
// static
+std::string ExtensionTabUtil::GetBrowserWindowTypeText(const Browser& browser) {
+ if (browser.is_devtools())
+ return keys::kWindowTypeValueDevTools;
+ if (browser.is_type_popup())
+ return keys::kWindowTypeValuePopup;
+ // TODO(devlin): Browser::is_app() returns true whenever Browser::app_name_
+ // is non-empty (and includes instances such as devtools). Platform apps
+ // should no longer be returned here; are there any other cases (that aren't
+ // captured by is_devtools() or is_type_popup() for an app-type browser?
+ if (browser.is_app())
+ return keys::kWindowTypeValueApp;
+ return keys::kWindowTypeValueNormal;
+}
+
+// static
std::unique_ptr<api::tabs::Tab> ExtensionTabUtil::CreateTabObject(
WebContents* contents,
TabStripModel* tab_strip,
int tab_index,
const Extension* extension) {
- // If we have a matching AppWindow with a controller, get the tab value
- // from its controller instead.
- WindowController* controller = GetAppWindowController(contents);
- if (controller) {
- DCHECK(!extension || controller->IsVisibleToTabsAPIForExtension(
- extension, false /*allow_dev_tools_windows*/));
- return controller->CreateTabObject(extension, tab_index);
- }
std::unique_ptr<api::tabs::Tab> result =
CreateTabObject(contents, tab_strip, tab_index);
ScrubTabForExtension(extension, contents, result.get());
@@ -366,12 +362,6 @@
content::WebContents* contents,
TabStripModel* tab_strip,
int tab_index) {
- // If we have a matching AppWindow with a controller, get the tab value
- // from its controller instead.
- WindowController* controller = GetAppWindowController(contents);
- if (controller)
- return controller->CreateTabObject(nullptr, tab_index);
-
if (!tab_strip)
ExtensionTabUtil::GetTabStripModel(contents, &tab_strip, &tab_index);
bool is_loading = contents->IsLoading();
@@ -416,6 +406,54 @@
}
// static
+std::unique_ptr<base::DictionaryValue>
+ExtensionTabUtil::CreateWindowValueForExtension(
+ const Browser& browser,
+ const Extension* extension,
+ PopulateTabBehavior populate_tab_behavior) {
+ auto result = std::make_unique<base::DictionaryValue>();
+
+ result->SetInteger(keys::kIdKey, browser.session_id().id());
+ result->SetString(keys::kWindowTypeKey, GetBrowserWindowTypeText(browser));
+ ui::BaseWindow* window = browser.window();
+ result->SetBoolean(keys::kFocusedKey, window->IsActive());
+ const Profile* profile = browser.profile();
+ result->SetBoolean(keys::kIncognitoKey, profile->IsOffTheRecord());
+ result->SetBoolean(keys::kAlwaysOnTopKey, window->IsAlwaysOnTop());
+
+ std::string window_state;
+ if (window->IsMinimized()) {
+ window_state = keys::kShowStateValueMinimized;
+ } else if (window->IsFullscreen()) {
+ window_state = keys::kShowStateValueFullscreen;
+#if defined(OS_CHROMEOS)
+ if (ash::IsWindowTrustedPinned(window))
+ window_state = keys::kShowStateValueLockedFullscreen;
+#endif
+ } else if (window->IsMaximized()) {
+ window_state = keys::kShowStateValueMaximized;
+ } else {
+ window_state = keys::kShowStateValueNormal;
+ }
+ result->SetString(keys::kShowStateKey, window_state);
+
+ gfx::Rect bounds;
+ if (window->IsMinimized())
+ bounds = window->GetRestoredBounds();
+ else
+ bounds = window->GetBounds();
+ result->SetInteger(keys::kLeftKey, bounds.x());
+ result->SetInteger(keys::kTopKey, bounds.y());
+ result->SetInteger(keys::kWidthKey, bounds.width());
+ result->SetInteger(keys::kHeightKey, bounds.height());
+
+ if (populate_tab_behavior == kPopulateTabs)
+ result->Set(keys::kTabsKey, CreateTabList(&browser, extension));
+
+ return result;
+}
+
+// static
std::unique_ptr<api::tabs::MutedInfo> ExtensionTabUtil::CreateMutedInfo(
content::WebContents* contents) {
DCHECK(contents);
diff --git a/chrome/browser/extensions/extension_tab_util.h b/chrome/browser/extensions/extension_tab_util.h
index d01ad91..64a4693e 100644
--- a/chrome/browser/extensions/extension_tab_util.h
+++ b/chrome/browser/extensions/extension_tab_util.h
@@ -39,6 +39,11 @@
// Provides various utility functions that help manipulate tabs.
class ExtensionTabUtil {
public:
+ enum PopulateTabBehavior {
+ kPopulateTabs,
+ kDontPopulateTabs,
+ };
+
struct OpenTabParams {
OpenTabParams();
~OpenTabParams();
@@ -84,6 +89,9 @@
int window_id,
std::string* error_message);
+ // Returns the tabs:: API constant for the window type of the |browser|.
+ static std::string GetBrowserWindowTypeText(const Browser& browser);
+
// Creates a Tab object (see chrome/common/extensions/api/tabs.json) with
// information about the state of a browser tab. Depending on the
// permissions of the extension, the object may or may not include sensitive
@@ -101,6 +109,9 @@
// Creates a Tab object but performs no extension permissions checks; the
// returned object will contain privacy-sensitive data.
+ // TODO(devlin): These are easy to confuse with the safer, info-scrubbing
+ // versions above. We should get rid of these, and have callers explicitly
+ // pass in a null extension if they do not want values scrubbed.
static std::unique_ptr<api::tabs::Tab> CreateTabObject(
content::WebContents* web_contents) {
return CreateTabObject(web_contents, nullptr, -1);
@@ -110,6 +121,16 @@
TabStripModel* tab_strip,
int tab_index);
+ // Creates a DictionaryValue representing the window for the given |browser|,
+ // and scrubs any privacy-sensitive data that |extension| does not have
+ // access to. |populate_tab_behavior| determines whether tabs will be
+ // populated in the result.
+ // TODO(devlin): Convert this to a api::Windows::Window object.
+ static std::unique_ptr<base::DictionaryValue> CreateWindowValueForExtension(
+ const Browser& browser,
+ const Extension* extension,
+ PopulateTabBehavior populate_tab_behavior);
+
// Creates a tab MutedInfo object (see chrome/common/extensions/api/tabs.json)
// with information about the mute state of a browser tab.
static std::unique_ptr<api::tabs::MutedInfo> CreateMutedInfo(
diff --git a/chrome/browser/extensions/window_controller.cc b/chrome/browser/extensions/window_controller.cc
index 05afc52..a736d8a 100644
--- a/chrome/browser/extensions/window_controller.cc
+++ b/chrome/browser/extensions/window_controller.cc
@@ -13,12 +13,6 @@
#include "chrome/browser/extensions/window_controller_list.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/extensions/api/windows.h"
-#include "ui/base/base_window.h"
-#include "ui/gfx/geometry/rect.h"
-
-#if defined(OS_CHROMEOS)
-#include "ash/public/cpp/window_pin_type.h"
-#endif
namespace extensions {
@@ -72,47 +66,6 @@
return NULL;
}
-namespace keys = tabs_constants;
-
-std::unique_ptr<base::DictionaryValue> WindowController::CreateWindowValue()
- const {
- std::unique_ptr<base::DictionaryValue> result(new base::DictionaryValue());
-
- result->SetInteger(keys::kIdKey, GetWindowId());
- result->SetString(keys::kWindowTypeKey, GetWindowTypeText());
- result->SetBoolean(keys::kFocusedKey, window()->IsActive());
- result->SetBoolean(keys::kIncognitoKey, profile_->IsOffTheRecord());
- result->SetBoolean(keys::kAlwaysOnTopKey, window()->IsAlwaysOnTop());
-
- std::string window_state;
- if (window()->IsMinimized()) {
- window_state = keys::kShowStateValueMinimized;
- } else if (window()->IsFullscreen()) {
- window_state = keys::kShowStateValueFullscreen;
-#if defined(OS_CHROMEOS)
- if (ash::IsWindowTrustedPinned(window()))
- window_state = keys::kShowStateValueLockedFullscreen;
-#endif
- } else if (window()->IsMaximized()) {
- window_state = keys::kShowStateValueMaximized;
- } else {
- window_state = keys::kShowStateValueNormal;
- }
- result->SetString(keys::kShowStateKey, window_state);
-
- gfx::Rect bounds;
- if (window()->IsMinimized())
- bounds = window()->GetRestoredBounds();
- else
- bounds = window()->GetBounds();
- result->SetInteger(keys::kLeftKey, bounds.x());
- result->SetInteger(keys::kTopKey, bounds.y());
- result->SetInteger(keys::kWidthKey, bounds.width());
- result->SetInteger(keys::kHeightKey, bounds.height());
-
- return result;
-}
-
bool WindowController::MatchesFilter(TypeFilter filter) const {
TypeFilter type = 1 << api::windows::ParseWindowType(GetWindowTypeText());
return (type & filter) != 0;
diff --git a/chrome/browser/extensions/window_controller.h b/chrome/browser/extensions/window_controller.h
index 941d8f65..b5380f7 100644
--- a/chrome/browser/extensions/window_controller.h
+++ b/chrome/browser/extensions/window_controller.h
@@ -9,6 +9,7 @@
#include <memory>
#include <string>
+#include <vector>
#include "base/compiler_specific.h"
#include "base/macros.h"
@@ -19,10 +20,6 @@
class GURL;
class Profile;
-namespace base {
-class DictionaryValue;
-}
-
namespace ui {
class BaseWindow;
}
@@ -31,7 +28,7 @@
class Extension;
// This API needs to be implemented by any window that might be accessed
-// through chrome.windows or chrome.tabs (e.g. browser windows and panels).
+// through various extension APIs for modifying or finding the window.
// Subclasses must add/remove themselves from the WindowControllerList
// upon construction/destruction.
class WindowController {
@@ -70,21 +67,9 @@
virtual int GetWindowId() const = 0;
// Return the type name for the window.
+ // TODO(devlin): Remove this in favor of the method on ExtensionTabUtil.
virtual std::string GetWindowTypeText() const = 0;
- // Populates a dictionary for the Window object. Override this to set
- // implementation specific properties (call the base implementation first to
- // set common properties).
- std::unique_ptr<base::DictionaryValue> CreateWindowValue() const;
-
- // Populates a dictionary for the Window object, including a list of tabs.
- virtual std::unique_ptr<base::DictionaryValue> CreateWindowValueWithTabs(
- const extensions::Extension* extension) const = 0;
-
- virtual std::unique_ptr<api::tabs::Tab> CreateTabObject(
- const extensions::Extension* extension,
- int tab_index) const = 0;
-
// Returns false if the window is in a state where closing the window is not
// permitted and sets |reason| if not NULL.
virtual bool CanClose(Reason* reason) const = 0;
