Skip to content

Commit

Permalink
Revert of Remove WINDOW_TYPE_V1_PANEL AppWindow type (patchset #2 id:…
Browse files Browse the repository at this point in the history
…20001 of https://codereview.chromium.org/2886293002/ )

Reason for revert:
Breaks compile on ChromiumOS x86-generic due to unused-but-set variable saw_focus_key

Original issue's description:
> Remove WINDOW_TYPE_V1_PANEL AppWindow type
>
> Changes:
> Currently, panel window is only used as Chrome OS's audio player, which is WINDOW_TYPE_PANEL. WINDOW_TYPE_V1_PANEL is not a AppWindow type in use any more. Remove it in code base.
>
> BUG=691099
> TEST=covered by tests
>
> Review-Url: https://codereview.chromium.org/2886293002
> Cr-Commit-Position: refs/heads/master@{#472661}
> Committed: https://chromium.googlesource.com/chromium/src/+/0775f98187d5d895b15b46212f55d30956aa946e

TBR=benwells@chromium.org,oshima@chromium.org,warx@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=691099

Review-Url: https://codereview.chromium.org/2891043002
Cr-Commit-Position: refs/heads/master@{#472740}
  • Loading branch information
treib authored and Commit bot committed May 18, 2017
1 parent ad3ffab commit 3cb8f0a
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 4 deletions.
43 changes: 43 additions & 0 deletions chrome/browser/extensions/api/tabs/tabs_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@
#include "ui/base/models/list_selection_model.h"
#include "ui/base/ui_base_types.h"

#if defined(USE_ASH)
#include "chrome/browser/extensions/api/tabs/ash_panel_contents.h" // nogncheck
#include "extensions/browser/app_window/app_window_registry.h" // nogncheck
#endif

using content::BrowserThread;
using content::NavigationController;
using content::NavigationEntry;
Expand Down Expand Up @@ -481,6 +486,7 @@ ExtensionFunction::ResponseAction WindowsCreateFunction::Run() {
Browser::Type window_type = Browser::TYPE_TABBED;

#if defined(USE_ASH)
bool create_ash_panel = false;
bool saw_focus_key = false;
#endif // defined(USE_ASH)

Expand All @@ -503,6 +509,18 @@ ExtensionFunction::ResponseAction WindowsCreateFunction::Run() {

case windows::CREATE_TYPE_PANEL: {
extension_id = extension()->id();
#if defined(USE_ASH)
// Only ChromeOS' version of chrome.windows.create would create a panel
// window. It is whitelisted to Hangouts extension for limited time until
// it transitioned to other types of windows.
for (const char* id : extension_misc::kHangoutsExtensionIds) {
if (extension_id == id) {
create_ash_panel = true;
break;
}
}
#endif // defined(USE_ASH)
// Everything else gets POPUP instead of PANEL.
// TODO(dimich): Eventually, remove the 'panel' values form valid
// window.create parameters. However, this is a more breaking change, so
// for now simply treat it as a POPUP.
Expand Down Expand Up @@ -555,6 +573,31 @@ ExtensionFunction::ResponseAction WindowsCreateFunction::Run() {
}
}

#if defined(USE_ASH)
if (create_ash_panel) {
if (urls.empty())
urls.push_back(GURL(chrome::kChromeUINewTabURL));

AppWindow::CreateParams create_params;
create_params.window_type = AppWindow::WINDOW_TYPE_V1_PANEL;
create_params.window_key = extension_id;
create_params.window_spec.bounds = window_bounds;
create_params.focused = saw_focus_key && focused;
AppWindow* app_window =
new AppWindow(window_profile, new ChromeAppDelegate(true), extension());
AshPanelContents* ash_panel_contents = new AshPanelContents(app_window);
app_window->Init(urls[0], ash_panel_contents, render_frame_host(),
create_params);
WindowController* window_controller =
WindowControllerList::GetInstance()->FindWindowById(
app_window->session_id().id());
if (!window_controller)
return RespondNow(Error(kUnknownErrorDoNotUse));
return RespondNow(
OneArgument(window_controller->CreateWindowValueWithTabs(extension())));
}
#endif // defined(USE_ASH)

// Create a new BrowserWindow.
Browser::CreateParams create_params(window_type, window_profile,
user_gesture());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,9 @@ void RecordUMAForTransferredWindowType(aura::Window* window) {
}
if (app_window) {
if (app_window->window_type() ==
extensions::AppWindow::WINDOW_TYPE_PANEL) {
extensions::AppWindow::WINDOW_TYPE_PANEL ||
app_window->window_type() ==
extensions::AppWindow::WINDOW_TYPE_V1_PANEL) {
window_type = ash::MultiProfileUMA::TELEPORT_WINDOW_PANEL;
} else {
window_type = ash::MultiProfileUMA::TELEPORT_WINDOW_V2_APP;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,8 @@ void ChromeNativeAppWindowViews::InitializeWindow(
has_frame_color_ = create_params.has_frame_color;
active_frame_color_ = create_params.active_frame_color;
inactive_frame_color_ = create_params.inactive_frame_color;
if (create_params.window_type == AppWindow::WINDOW_TYPE_PANEL) {
if (create_params.window_type == AppWindow::WINDOW_TYPE_PANEL ||
create_params.window_type == AppWindow::WINDOW_TYPE_V1_PANEL) {
InitializePanelWindow(create_params);
} else {
InitializeDefaultWindow(create_params);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ void ChromeNativeAppWindowViewsAuraAsh::OnBeforeWidgetInit(
}
}
DCHECK_NE(AppWindow::WINDOW_TYPE_PANEL, create_params.window_type);
DCHECK_NE(AppWindow::WINDOW_TYPE_V1_PANEL, create_params.window_type);
init_params->mus_properties
[ui::mojom::WindowManager::kRemoveStandardFrame_InitProperty] =
mojo::ConvertTo<std::vector<uint8_t>>(init_params->remove_standard_frame);
Expand Down
5 changes: 4 additions & 1 deletion extensions/browser/app_window/app_window.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ class AppWindow : public content::WebContentsDelegate,
enum WindowType {
WINDOW_TYPE_DEFAULT = 1 << 0, // Default app window.
WINDOW_TYPE_PANEL = 1 << 1, // OS controlled panel window (Ash only).
WINDOW_TYPE_V1_PANEL = 1 << 2, // For apps v1 support in Ash; deprecate
// with v1 apps.
};

enum Frame {
Expand Down Expand Up @@ -233,7 +235,8 @@ class AppWindow : public content::WebContentsDelegate,
content::WebContents* web_contents() const;
WindowType window_type() const { return window_type_; }
bool window_type_is_panel() const {
return window_type_ == WINDOW_TYPE_PANEL;
return (window_type_ == WINDOW_TYPE_PANEL ||
window_type_ == WINDOW_TYPE_V1_PANEL);
}
content::BrowserContext* browser_context() const { return browser_context_; }
const gfx::Image& app_icon() const { return app_icon_; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,11 @@ base::string16 NativeAppWindowViews::GetWindowTitle() const {
}

bool NativeAppWindowViews::ShouldShowWindowTitle() const {
return false;
return app_window_->window_type() == AppWindow::WINDOW_TYPE_V1_PANEL;
}

bool NativeAppWindowViews::ShouldShowWindowIcon() const {
return app_window_->window_type() == AppWindow::WINDOW_TYPE_V1_PANEL;
}

void NativeAppWindowViews::SaveWindowPlacement(const gfx::Rect& bounds,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ class NativeAppWindowViews : public extensions::NativeAppWindow,
bool CanMinimize() const override;
base::string16 GetWindowTitle() const override;
bool ShouldShowWindowTitle() const override;
bool ShouldShowWindowIcon() const override;
void SaveWindowPlacement(const gfx::Rect& bounds,
ui::WindowShowState show_state) override;
void DeleteDelegate() override;
Expand Down

0 comments on commit 3cb8f0a

Please sign in to comment.