Skip to content
This repository was archived by the owner on Apr 3, 2020. It is now read-only.

Commit 7543ca1

Browse files
committed
Revert "Add AppWindow.setVisibleOnAllWorkspaces." (https://codereview.chromium.org/469993003)
Speculative, to see if it helps with ==12563== WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x7f834414fd2c in views::DesktopWindowTreeHostX11::InitX11Window(views::Widget::InitParams const&) ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1164 #1 0x7f834414dc6e in views::DesktopWindowTreeHostX11::Init(aura::Window*, views::Widget::InitParams const&) ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:259 #2 0x7f83441408e9 in views::DesktopNativeWidgetAura::InitNativeWidget(views::Widget::InitParams const&) ui/views/widget/desktop_aura/desktop_native_widget_aura.cc:420 #3 0x7f834411722f in views::Widget::Init(views::Widget::InitParams const&) ui/views/widget/widget.cc:358 #4 0x7f834473bc8b in ChromeNativeAppWindowViews::InitializeDefaultWindow(extensions::AppWindow::CreateParams const&) chrome/browser/ui/views/apps/chrome_native_app_window_views.cc:231 #5 0x7f834473f52d in ChromeNativeAppWindowViews::InitializeWindow(extensions::AppWindow*, extensions::AppWindow::CreateParams const&) chrome/browser/ui/views/apps/chrome_native_app_window_views.cc:672 #6 0x7f834c3d8bbe in apps::NativeAppWindowViews::Init(extensions::AppWindow*, extensions::AppWindow::CreateParams const&) apps/ui/views/native_app_window_views.cc:46 #7 0x7f834460443c in ChromeAppsClient::CreateNativeAppWindowImpl(extensions::AppWindow*, extensions::AppWindow::CreateParams const&) chrome/browser/ui/views/apps/chrome_apps_client_views.cc:14 #8 0x7f834be98433 in extensions::AppWindow::Init(GURL const&, extensions::AppWindowContents*, extensions::AppWindow::CreateParams const&) extensions/browser/app_window/app_window.cc:281 #9 0x7f834bd6bbfb in extensions::AppWindowCreateFunction::RunAsync() extensions/browser/api/app_window/app_window_api.cc:296 #10 0x7f834bee4510 in AsyncExtensionFunction::Run() extensions/browser/extension_function.cc:452 #11 0x7f834bee9771 in extensions::ExtensionFunctionDispatcher::DispatchWithCallbackInternal(ExtensionHostMsg_Request_Params const&, content::RenderViewHost*, content::RenderFrameHost*, base::Callback<void (ExtensionFunction::ResponseType, base::ListValue const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)> const&) extensions/browser/extension_function_dispatcher.cc:392 #12 0x7f834bee86b6 in extensions::ExtensionFunctionDispatcher::Dispatch(ExtensionHostMsg_Request_Params const&, content::RenderViewHost*) extensions/browser/extension_function_dispatcher.cc:309 #13 0x7f834bedac91 in OnRequest extensions/browser/extension_host.cc:342 #14 0x7f8345add9d2 in content::WebContentsImpl::OnMessageReceived(content::RenderViewHost*, content::RenderFrameHost*, IPC::Message const&) content/browser/web_contents/web_contents_impl.cc:526 #15 0x7f83459847fe in content::RenderViewHostImpl::OnMessageReceived(IPC::Message const&) content/browser/renderer_host/render_view_host_impl.cc:894 Started happening here http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20MSan%20Tests/builds/98 (see stdio for symbols), and this looks like the only change in that build. This reverts commit c76ef73. BUG=384644 [email protected] Review URL: https://codereview.chromium.org/550413002 Cr-Commit-Position: refs/heads/master@{#293792}
1 parent a0d051e commit 7543ca1

File tree

24 files changed

+0
-164
lines changed

24 files changed

+0
-164
lines changed

apps/ui/views/native_app_window_views.cc

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -401,8 +401,4 @@ bool NativeAppWindowViews::CanHaveAlphaEnabled() const {
401401
return widget_->IsTranslucentWindowOpacitySupported();
402402
}
403403

404-
void NativeAppWindowViews::SetVisibleOnAllWorkspaces(bool always_visible) {
405-
widget_->SetVisibleOnAllWorkspaces(always_visible);
406-
}
407-
408404
} // namespace apps

apps/ui/views/native_app_window_views.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,6 @@ class NativeAppWindowViews : public extensions::NativeAppWindow,
161161
virtual void SetContentSizeConstraints(const gfx::Size& min_size,
162162
const gfx::Size& max_size) OVERRIDE;
163163
virtual bool CanHaveAlphaEnabled() const OVERRIDE;
164-
virtual void SetVisibleOnAllWorkspaces(bool always_visible) OVERRIDE;
165164

166165
// web_modal::WebContentsModalDialogHost implementation.
167166
virtual gfx::NativeView GetHostView() const OVERRIDE;

chrome/browser/apps/app_window_browsertest.cc

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -257,9 +257,3 @@ IN_PROC_BROWSER_TEST_F(AppWindowAPITest, TestFrameColorsInStable) {
257257
ASSERT_TRUE(RunAppWindowAPITest("testFrameColors")) << message_;
258258
}
259259
#endif
260-
261-
IN_PROC_BROWSER_TEST_F(AppWindowAPITest, TestVisibleOnAllWorkspaces) {
262-
ASSERT_TRUE(
263-
RunAppWindowAPITestAndWaitForRoundTrip("testVisibleOnAllWorkspaces"))
264-
<< message_;
265-
}

chrome/browser/extensions/api/app_current_window_internal/app_current_window_internal_api.cc

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@ namespace SetIcon = app_current_window_internal::SetIcon;
2727
namespace SetBadgeIcon = app_current_window_internal::SetBadgeIcon;
2828
namespace SetShape = app_current_window_internal::SetShape;
2929
namespace SetAlwaysOnTop = app_current_window_internal::SetAlwaysOnTop;
30-
namespace SetVisibleOnAllWorkspaces =
31-
app_current_window_internal::SetVisibleOnAllWorkspaces;
3230

3331
using app_current_window_internal::Bounds;
3432
using app_current_window_internal::Region;
@@ -399,18 +397,4 @@ bool AppCurrentWindowInternalSetAlwaysOnTopFunction::RunWithWindow(
399397
return true;
400398
}
401399

402-
bool AppCurrentWindowInternalSetVisibleOnAllWorkspacesFunction::RunWithWindow(
403-
AppWindow* window) {
404-
if (GetCurrentChannel() > chrome::VersionInfo::CHANNEL_DEV) {
405-
error_ = kDevChannelOnly;
406-
return false;
407-
}
408-
409-
scoped_ptr<SetVisibleOnAllWorkspaces::Params> params(
410-
SetVisibleOnAllWorkspaces::Params::Create(*args_));
411-
CHECK(params.get());
412-
window->GetBaseWindow()->SetVisibleOnAllWorkspaces(params->always_visible);
413-
return true;
414-
}
415-
416400
} // namespace extensions

chrome/browser/extensions/api/app_current_window_internal/app_current_window_internal_api.h

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -197,18 +197,6 @@ class AppCurrentWindowInternalSetAlwaysOnTopFunction
197197
virtual bool RunWithWindow(AppWindow* window) OVERRIDE;
198198
};
199199

200-
class AppCurrentWindowInternalSetVisibleOnAllWorkspacesFunction
201-
: public AppCurrentWindowInternalExtensionFunction {
202-
public:
203-
DECLARE_EXTENSION_FUNCTION(
204-
"app.currentWindowInternal.setVisibleOnAllWorkspaces",
205-
APP_CURRENTWINDOWINTERNAL_SETVISIBLEONALLWORKSPACES)
206-
207-
protected:
208-
virtual ~AppCurrentWindowInternalSetVisibleOnAllWorkspacesFunction() {}
209-
virtual bool RunWithWindow(AppWindow* window) OVERRIDE;
210-
};
211-
212200
} // namespace extensions
213201

214202
#endif // CHROME_BROWSER_EXTENSIONS_API_APP_CURRENT_WINDOW_INTERNAL_APP_CURRENT_WINDOW_INTERNAL_API_H_

chrome/browser/ui/cocoa/apps/native_app_window_cocoa.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,6 @@ class NativeAppWindowCocoa : public extensions::NativeAppWindow,
148148
virtual gfx::Size GetContentMaximumSize() const OVERRIDE;
149149
virtual void SetContentSizeConstraints(const gfx::Size& min_size,
150150
const gfx::Size& max_size) OVERRIDE;
151-
virtual void SetVisibleOnAllWorkspaces(bool always_visible) OVERRIDE;
152151

153152
// WebContentsObserver implementation.
154153
virtual void RenderViewCreated(content::RenderViewHost* rvh) OVERRIDE;

chrome/browser/ui/cocoa/apps/native_app_window_cocoa.mm

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -60,15 +60,6 @@ void SetFullScreenCollectionBehavior(NSWindow* window, bool allow_fullscreen) {
6060
[window setCollectionBehavior:behavior];
6161
}
6262

63-
void SetWorkspacesCollectionBehavior(NSWindow* window, bool always_visible) {
64-
NSWindowCollectionBehavior behavior = [window collectionBehavior];
65-
if (always_visible)
66-
behavior |= NSWindowCollectionBehaviorCanJoinAllSpaces;
67-
else
68-
behavior &= ~NSWindowCollectionBehaviorCanJoinAllSpaces;
69-
[window setCollectionBehavior:behavior];
70-
}
71-
7263
void InitCollectionBehavior(NSWindow* window) {
7364
// Since always-on-top windows have a higher window level
7465
// than NSNormalWindowLevel, they will default to
@@ -388,8 +379,6 @@ - (void)setMouseDownCanMoveWindow:(BOOL)can_move;
388379
[window setLevel:AlwaysOnTopWindowLevel()];
389380
InitCollectionBehavior(window);
390381

391-
SetWorkspacesCollectionBehavior(window, params.visible_on_all_workspaces);
392-
393382
window_controller_.reset(
394383
[[NativeAppWindowController alloc] initWithWindow:window.release()]);
395384

@@ -978,10 +967,6 @@ - (void)setMouseDownCanMoveWindow:(BOOL)can_move;
978967
NSNormalWindowLevel)];
979968
}
980969

981-
void NativeAppWindowCocoa::SetVisibleOnAllWorkspaces(bool always_visible) {
982-
SetWorkspacesCollectionBehavior(window(), always_visible);
983-
}
984-
985970
NativeAppWindowCocoa::~NativeAppWindowCocoa() {
986971
}
987972

chrome/browser/ui/views/apps/chrome_native_app_window_views.cc

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,6 @@ void ChromeNativeAppWindowViews::InitializeDefaultWindow(
215215
if (create_params.alpha_enabled)
216216
init_params.opacity = views::Widget::InitParams::TRANSLUCENT_WINDOW;
217217
init_params.keep_on_top = create_params.always_on_top;
218-
init_params.visible_on_all_workspaces =
219-
create_params.visible_on_all_workspaces;
220218

221219
#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
222220
// Set up a custom WM_CLASS for app windows. This allows task switchers in

chrome/common/extensions/api/_api_features.json

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,6 @@
7575
"extension_types": ["platform_app"],
7676
"noparent": true
7777
},
78-
"app.window.canSetVisibleOnAllWorkspaces": {
79-
"channel": "dev"
80-
},
8178
"app.currentWindowInternal": {
8279
"noparent": true,
8380
"internal": true,

chrome/common/extensions/api/app_current_window_internal.idl

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
static void clearBadge();
5353
static void setShape(Region region);
5454
static void setAlwaysOnTop(boolean always_on_top);
55-
static void setVisibleOnAllWorkspaces(boolean always_visible);
5655
};
5756

5857
interface Events {

chrome/test/data/extensions/platform_apps/window_api/test.js

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1352,21 +1352,6 @@ function testFrameColors() {
13521352
]);
13531353
}
13541354

1355-
function testVisibleOnAllWorkspaces() {
1356-
chrome.test.runTests([
1357-
function setAndUnsetVisibleOnAllWorkspaces() {
1358-
chrome.app.window.create('test.html', {
1359-
visibleOnAllWorkspaces: true
1360-
}, callbackPass(function(win) {
1361-
win.setVisibleOnAllWorkspaces(false);
1362-
win.setVisibleOnAllWorkspaces(true);
1363-
chrome.test.sendMessage(
1364-
'WaitForRoundTrip', callbackPass(function(reply) {}));
1365-
}));
1366-
},
1367-
]);
1368-
}
1369-
13701355
chrome.app.runtime.onLaunched.addListener(function() {
13711356
chrome.test.sendMessage('Launched', function(reply) {
13721357
window[reply]();

chrome/test/data/extensions/platform_apps/windows_api_visible_on_all_workspaces/in_stable/background.js

Lines changed: 0 additions & 28 deletions
This file was deleted.

chrome/test/data/extensions/platform_apps/windows_api_visible_on_all_workspaces/in_stable/index.html

Lines changed: 0 additions & 1 deletion
This file was deleted.

chrome/test/data/extensions/platform_apps/windows_api_visible_on_all_workspaces/in_stable/manifest.json

Lines changed: 0 additions & 10 deletions
This file was deleted.

extensions/browser/api/app_window/app_window_api.cc

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,6 @@ const char kAlphaEnabledMissingPermission[] =
5656
"The alphaEnabled option requires app.window.alpha permission.";
5757
const char kAlphaEnabledNeedsFrameNone[] =
5858
"The alphaEnabled option can only be used with \"frame: 'none'\".";
59-
const char kVisibleOnAllWorkspacesWrongChannel[] =
60-
"The visibleOnAllWorkspaces option requires dev channel or newer.";
6159
} // namespace app_window_constants
6260

6361
const char kNoneFrameOption[] = "none";
@@ -261,15 +259,6 @@ bool AppWindowCreateFunction::RunAsync() {
261259
if (options->focused.get())
262260
create_params.focused = *options->focused.get();
263261

264-
if (options->visible_on_all_workspaces.get()) {
265-
if (AppsClient::Get()->IsCurrentChannelOlderThanDev()) {
266-
error_ = app_window_constants::kVisibleOnAllWorkspacesWrongChannel;
267-
return false;
268-
}
269-
create_params.visible_on_all_workspaces =
270-
*options->visible_on_all_workspaces.get();
271-
}
272-
273262
if (options->type != app_window::WINDOW_TYPE_PANEL) {
274263
switch (options->state) {
275264
case app_window::STATE_NONE:

extensions/browser/api/app_window/app_window_apitest.cc

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -160,12 +160,4 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest,
160160
<< message_;
161161
}
162162

163-
IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest,
164-
WindowsApiVisibleOnAllWorkspacesInStable) {
165-
extensions::ScopedCurrentChannel channel(chrome::VersionInfo::CHANNEL_STABLE);
166-
EXPECT_TRUE(RunPlatformAppTest(
167-
"platform_apps/windows_api_visible_on_all_workspaces/in_stable"))
168-
<< message_;
169-
}
170-
171163
} // namespace extensions

extensions/browser/app_window/app_window.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -180,9 +180,6 @@ class AppWindow : public content::NotificationObserver,
180180
// configured to be always on top. Defaults to false.
181181
bool always_on_top;
182182

183-
// If true, the window will be visible on all workspaces. Defaults to false.
184-
bool visible_on_all_workspaces;
185-
186183
// The API enables developers to specify content or window bounds. This
187184
// function combines them into a single, constrained window size.
188185
gfx::Rect GetInitialWindowBounds(const gfx::Insets& frame_insets) const;

extensions/browser/app_window/native_app_window.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,6 @@ class NativeAppWindow : public ui::BaseWindow,
9292
virtual void SetContentSizeConstraints(const gfx::Size& min_size,
9393
const gfx::Size& max_size) = 0;
9494

95-
// Returns whether the window show be visible on all workspaces.
96-
virtual void SetVisibleOnAllWorkspaces(bool always_visible) = 0;
97-
9895
// Returns false if the underlying native window ignores alpha transparency
9996
// when compositing.
10097
virtual bool CanHaveAlphaEnabled() const = 0;

extensions/browser/extension_function_histogram_value.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -948,7 +948,6 @@ enum HistogramValue {
948948
MEDIAGALLERIES_GETALLGALLERYWATCH,
949949
MEDIAGALLERIES_REMOVEALLGALLERYWATCH,
950950
MANAGEMENT_GETSELF,
951-
APP_CURRENTWINDOWINTERNAL_SETVISIBLEONALLWORKSPACES,
952951
// Last entry: Add new entries above and ensure to update
953952
// tools/metrics/histograms/histograms.xml.
954953
ENUM_BOUNDARY

extensions/common/api/app_window.idl

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -263,10 +263,6 @@ namespace app.window {
263263

264264
// If true, the window will be focused when created. Defaults to true.
265265
boolean? focused;
266-
267-
// If true, the window will be visible on all workspaces.
268-
// This is only available on dev channel.
269-
boolean? visibleOnAllWorkspaces;
270266
};
271267

272268
// Called in the creating window (parent) before the load event is called in
@@ -373,11 +369,6 @@ namespace app.window {
373369
// TODO(jackhou): Document this properly before going to stable.
374370
[nodoc] static boolean alphaEnabled();
375371

376-
// For platforms that support multiple workspaces, is this window visible on
377-
// all of them?
378-
// This is only available on dev channel.
379-
static void setVisibleOnAllWorkspaces(boolean alwaysVisible);
380-
381372
// The JavaScript 'window' object for the created child.
382373
[instanceOf=Window] object contentWindow;
383374

@@ -435,10 +426,6 @@ namespace app.window {
435426
// Gets an $(ref:AppWindow) with the given id. If no window with the given id
436427
// exists null is returned. This method is new in Chrome 33.
437428
[nocompile] static AppWindow get(DOMString id);
438-
439-
// Does the current platform support windows being visible on all
440-
// workspaces?
441-
[nocompile] static boolean canSetVisibleOnAllWorkspaces();
442429
};
443430

444431
interface Events {

extensions/renderer/resources/app_window_custom_bindings.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -193,10 +193,6 @@ appWindow.registerCustomHook(function(bindingsAPI) {
193193
return windows.length > 0 ? windows[0] : null;
194194
});
195195

196-
apiFunctions.setHandleRequest('canSetVisibleOnAllWorkspaces', function() {
197-
return /Mac/.test(navigator.platform) || /Linux/.test(navigator.userAgent);
198-
});
199-
200196
// This is an internal function, but needs to be bound into a closure
201197
// so the correct JS context is used for global variables such as
202198
// currentWindowInternal, appWindowData, etc.

extensions/shell/browser/shell_native_app_window.cc

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -238,10 +238,6 @@ void ShellNativeAppWindow::SetContentSizeConstraints(
238238
NOTIMPLEMENTED();
239239
}
240240

241-
void ShellNativeAppWindow::SetVisibleOnAllWorkspaces(bool always_visible) {
242-
NOTIMPLEMENTED();
243-
}
244-
245241
bool ShellNativeAppWindow::CanHaveAlphaEnabled() const {
246242
NOTIMPLEMENTED();
247243
return false;

extensions/shell/browser/shell_native_app_window.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ class ShellNativeAppWindow : public NativeAppWindow {
7575
virtual gfx::Size GetContentMaximumSize() const OVERRIDE;
7676
virtual void SetContentSizeConstraints(const gfx::Size& min_size,
7777
const gfx::Size& max_size) OVERRIDE;
78-
virtual void SetVisibleOnAllWorkspaces(bool always_visible) OVERRIDE;
7978
virtual bool CanHaveAlphaEnabled() const OVERRIDE;
8079

8180
private:

tools/metrics/histograms/histograms.xml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41408,7 +41408,6 @@ Therefore, the affected-histogram name has to have at least one dot in it.
4140841408
<int value="887" label="MEDIAGALLERIES_GETALLGALLERYWATCH"/>
4140941409
<int value="888" label="MEDIAGALLERIES_REMOVEALLGALLERYWATCH"/>
4141041410
<int value="889" label="MANAGEMENT_GETSELF"/>
41411-
<int value="890" label="APP_CURRENTWINDOWINTERNAL_SETVISIBLEONALLWORKSPACES"/>
4141241411
</enum>
4141341412

4141441413
<enum name="ExtensionInstallCause" type="int">

0 commit comments

Comments
 (0)