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}
crosswalk-project · Sep 8, 2014 · 7543ca1 · 7543ca1
1 parent a0d051e
commit 7543ca1
Show file tree

Hide file tree

Showing 24 changed files with 0 additions and 164 deletions.
diff --git a/apps/ui/views/native_app_window_views.cc b/apps/ui/views/native_app_window_views.cc
@@ -401,8 +401,4 @@ bool NativeAppWindowViews::CanHaveAlphaEnabled() const {
   return widget_->IsTranslucentWindowOpacitySupported();
 }
 
-void NativeAppWindowViews::SetVisibleOnAllWorkspaces(bool always_visible) {
-  widget_->SetVisibleOnAllWorkspaces(always_visible);
-}
-
 }  // namespace apps
diff --git a/apps/ui/views/native_app_window_views.h b/apps/ui/views/native_app_window_views.h
@@ -161,7 +161,6 @@ class NativeAppWindowViews : public extensions::NativeAppWindow,
   virtual void SetContentSizeConstraints(const gfx::Size& min_size,
                                          const gfx::Size& max_size) OVERRIDE;
   virtual bool CanHaveAlphaEnabled() const OVERRIDE;
-  virtual void SetVisibleOnAllWorkspaces(bool always_visible) OVERRIDE;
 
   // web_modal::WebContentsModalDialogHost implementation.
   virtual gfx::NativeView GetHostView() const OVERRIDE;

diff --git a/chrome/browser/apps/app_window_browsertest.cc b/chrome/browser/apps/app_window_browsertest.cc
@@ -257,9 +257,3 @@ IN_PROC_BROWSER_TEST_F(AppWindowAPITest, TestFrameColorsInStable) {
   ASSERT_TRUE(RunAppWindowAPITest("testFrameColors")) << message_;
 }
 #endif
-
-IN_PROC_BROWSER_TEST_F(AppWindowAPITest, TestVisibleOnAllWorkspaces) {
-  ASSERT_TRUE(
-      RunAppWindowAPITestAndWaitForRoundTrip("testVisibleOnAllWorkspaces"))
-      << message_;
-}
diff --git a/chrome/browser/extensions/api/app_current_window_internal/app_current_window_internal_api.cc b/chrome/browser/extensions/api/app_current_window_internal/app_current_window_internal_api.cc
@@ -27,8 +27,6 @@ namespace SetIcon = app_current_window_internal::SetIcon;
 namespace SetBadgeIcon = app_current_window_internal::SetBadgeIcon;
 namespace SetShape = app_current_window_internal::SetShape;
 namespace SetAlwaysOnTop = app_current_window_internal::SetAlwaysOnTop;
-namespace SetVisibleOnAllWorkspaces =
-    app_current_window_internal::SetVisibleOnAllWorkspaces;
 
 using app_current_window_internal::Bounds;
 using app_current_window_internal::Region;
@@ -399,18 +397,4 @@ bool AppCurrentWindowInternalSetAlwaysOnTopFunction::RunWithWindow(
   return true;
 }
 
-bool AppCurrentWindowInternalSetVisibleOnAllWorkspacesFunction::RunWithWindow(
-    AppWindow* window) {
-  if (GetCurrentChannel() > chrome::VersionInfo::CHANNEL_DEV) {
-    error_ = kDevChannelOnly;
-    return false;
-  }
-
-  scoped_ptr<SetVisibleOnAllWorkspaces::Params> params(
-      SetVisibleOnAllWorkspaces::Params::Create(*args_));
-  CHECK(params.get());
-  window->GetBaseWindow()->SetVisibleOnAllWorkspaces(params->always_visible);
-  return true;
-}
-
 }  // namespace extensions
diff --git a/chrome/browser/extensions/api/app_current_window_internal/app_current_window_internal_api.h b/chrome/browser/extensions/api/app_current_window_internal/app_current_window_internal_api.h
@@ -197,18 +197,6 @@ class AppCurrentWindowInternalSetAlwaysOnTopFunction
   virtual bool RunWithWindow(AppWindow* window) OVERRIDE;
 };
 
-class AppCurrentWindowInternalSetVisibleOnAllWorkspacesFunction
-    : public AppCurrentWindowInternalExtensionFunction {
- public:
-  DECLARE_EXTENSION_FUNCTION(
-      "app.currentWindowInternal.setVisibleOnAllWorkspaces",
-      APP_CURRENTWINDOWINTERNAL_SETVISIBLEONALLWORKSPACES)
-
- protected:
-  virtual ~AppCurrentWindowInternalSetVisibleOnAllWorkspacesFunction() {}
-  virtual bool RunWithWindow(AppWindow* window) OVERRIDE;
-};
-
 }  // namespace extensions
 
 #endif  // CHROME_BROWSER_EXTENSIONS_API_APP_CURRENT_WINDOW_INTERNAL_APP_CURRENT_WINDOW_INTERNAL_API_H_
diff --git a/chrome/browser/ui/cocoa/apps/native_app_window_cocoa.h b/chrome/browser/ui/cocoa/apps/native_app_window_cocoa.h
@@ -148,7 +148,6 @@ class NativeAppWindowCocoa : public extensions::NativeAppWindow,
   virtual gfx::Size GetContentMaximumSize() const OVERRIDE;
   virtual void SetContentSizeConstraints(const gfx::Size& min_size,
                                          const gfx::Size& max_size) OVERRIDE;
-  virtual void SetVisibleOnAllWorkspaces(bool always_visible) OVERRIDE;
 
   // WebContentsObserver implementation.
   virtual void RenderViewCreated(content::RenderViewHost* rvh) OVERRIDE;

diff --git a/chrome/browser/ui/cocoa/apps/native_app_window_cocoa.mm b/chrome/browser/ui/cocoa/apps/native_app_window_cocoa.mm
@@ -60,15 +60,6 @@ void SetFullScreenCollectionBehavior(NSWindow* window, bool allow_fullscreen) {
   [window setCollectionBehavior:behavior];
 }
 
-void SetWorkspacesCollectionBehavior(NSWindow* window, bool always_visible) {
-  NSWindowCollectionBehavior behavior = [window collectionBehavior];
-  if (always_visible)
-    behavior |= NSWindowCollectionBehaviorCanJoinAllSpaces;
-  else
-    behavior &= ~NSWindowCollectionBehaviorCanJoinAllSpaces;
-  [window setCollectionBehavior:behavior];
-}
-
 void InitCollectionBehavior(NSWindow* window) {
   // Since always-on-top windows have a higher window level
   // than NSNormalWindowLevel, they will default to
@@ -388,8 +379,6 @@ - (void)setMouseDownCanMoveWindow:(BOOL)can_move;
     [window setLevel:AlwaysOnTopWindowLevel()];
   InitCollectionBehavior(window);
 
-  SetWorkspacesCollectionBehavior(window, params.visible_on_all_workspaces);
-
   window_controller_.reset(
       [[NativeAppWindowController alloc] initWithWindow:window.release()]);
 
@@ -978,10 +967,6 @@ - (void)setMouseDownCanMoveWindow:(BOOL)can_move;
                                       NSNormalWindowLevel)];
 }
 
-void NativeAppWindowCocoa::SetVisibleOnAllWorkspaces(bool always_visible) {
-  SetWorkspacesCollectionBehavior(window(), always_visible);
-}
-
 NativeAppWindowCocoa::~NativeAppWindowCocoa() {
 }
 

diff --git a/chrome/browser/ui/views/apps/chrome_native_app_window_views.cc b/chrome/browser/ui/views/apps/chrome_native_app_window_views.cc
@@ -215,8 +215,6 @@ void ChromeNativeAppWindowViews::InitializeDefaultWindow(
   if (create_params.alpha_enabled)
     init_params.opacity = views::Widget::InitParams::TRANSLUCENT_WINDOW;
   init_params.keep_on_top = create_params.always_on_top;
-  init_params.visible_on_all_workspaces =
-      create_params.visible_on_all_workspaces;
 
 #if defined(OS_LINUX) && !defined(OS_CHROMEOS)
   // Set up a custom WM_CLASS for app windows. This allows task switchers in

diff --git a/chrome/common/extensions/api/_api_features.json b/chrome/common/extensions/api/_api_features.json
@@ -75,9 +75,6 @@
     "extension_types": ["platform_app"],
     "noparent": true
   },
-  "app.window.canSetVisibleOnAllWorkspaces": {
-    "channel": "dev"
-  },
   "app.currentWindowInternal": {
     "noparent": true,
     "internal": true,

diff --git a/chrome/common/extensions/api/app_current_window_internal.idl b/chrome/common/extensions/api/app_current_window_internal.idl
@@ -52,7 +52,6 @@
     static void clearBadge();
     static void setShape(Region region);
     static void setAlwaysOnTop(boolean always_on_top);
-    static void setVisibleOnAllWorkspaces(boolean always_visible);
   };
 
   interface Events {

diff --git a/chrome/test/data/extensions/platform_apps/window_api/test.js b/chrome/test/data/extensions/platform_apps/window_api/test.js
@@ -1352,21 +1352,6 @@ function testFrameColors() {
   ]);
 }
 
-function testVisibleOnAllWorkspaces() {
-  chrome.test.runTests([
-    function setAndUnsetVisibleOnAllWorkspaces() {
-      chrome.app.window.create('test.html', {
-        visibleOnAllWorkspaces: true
-      }, callbackPass(function(win) {
-        win.setVisibleOnAllWorkspaces(false);
-        win.setVisibleOnAllWorkspaces(true);
-        chrome.test.sendMessage(
-            'WaitForRoundTrip', callbackPass(function(reply) {}));
-      }));
-    },
-  ]);
-}
-
 chrome.app.runtime.onLaunched.addListener(function() {
   chrome.test.sendMessage('Launched', function(reply) {
     window[reply]();

diff --git a/...ta/extensions/platform_apps/windows_api_visible_on_all_workspaces/in_stable/background.js b/...ta/extensions/platform_apps/windows_api_visible_on_all_workspaces/in_stable/background.js
diff --git a/.../data/extensions/platform_apps/windows_api_visible_on_all_workspaces/in_stable/index.html b/.../data/extensions/platform_apps/windows_api_visible_on_all_workspaces/in_stable/index.html
diff --git a/...ta/extensions/platform_apps/windows_api_visible_on_all_workspaces/in_stable/manifest.json b/...ta/extensions/platform_apps/windows_api_visible_on_all_workspaces/in_stable/manifest.json
diff --git a/extensions/browser/api/app_window/app_window_api.cc b/extensions/browser/api/app_window/app_window_api.cc
@@ -56,8 +56,6 @@ const char kAlphaEnabledMissingPermission[] =
     "The alphaEnabled option requires app.window.alpha permission.";
 const char kAlphaEnabledNeedsFrameNone[] =
     "The alphaEnabled option can only be used with \"frame: 'none'\".";
-const char kVisibleOnAllWorkspacesWrongChannel[] =
-    "The visibleOnAllWorkspaces option requires dev channel or newer.";
 }  // namespace app_window_constants
 
 const char kNoneFrameOption[] = "none";
@@ -261,15 +259,6 @@ bool AppWindowCreateFunction::RunAsync() {
     if (options->focused.get())
       create_params.focused = *options->focused.get();
 
-    if (options->visible_on_all_workspaces.get()) {
-      if (AppsClient::Get()->IsCurrentChannelOlderThanDev()) {
-        error_ = app_window_constants::kVisibleOnAllWorkspacesWrongChannel;
-        return false;
-      }
-      create_params.visible_on_all_workspaces =
-          *options->visible_on_all_workspaces.get();
-    }
-
     if (options->type != app_window::WINDOW_TYPE_PANEL) {
       switch (options->state) {
         case app_window::STATE_NONE:

diff --git a/extensions/browser/api/app_window/app_window_apitest.cc b/extensions/browser/api/app_window/app_window_apitest.cc
@@ -160,12 +160,4 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest,
       << message_;
 }
 
-IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest,
-                       WindowsApiVisibleOnAllWorkspacesInStable) {
-  extensions::ScopedCurrentChannel channel(chrome::VersionInfo::CHANNEL_STABLE);
-  EXPECT_TRUE(RunPlatformAppTest(
-      "platform_apps/windows_api_visible_on_all_workspaces/in_stable"))
-      << message_;
-}
-
 }  // namespace extensions
diff --git a/extensions/browser/app_window/app_window.h b/extensions/browser/app_window/app_window.h
@@ -180,9 +180,6 @@ class AppWindow : public content::NotificationObserver,
     // configured to be always on top. Defaults to false.
     bool always_on_top;
 
-    // If true, the window will be visible on all workspaces. Defaults to false.
-    bool visible_on_all_workspaces;
-
     // The API enables developers to specify content or window bounds. This
     // function combines them into a single, constrained window size.
     gfx::Rect GetInitialWindowBounds(const gfx::Insets& frame_insets) const;

diff --git a/extensions/browser/app_window/native_app_window.h b/extensions/browser/app_window/native_app_window.h
@@ -92,9 +92,6 @@ class NativeAppWindow : public ui::BaseWindow,
   virtual void SetContentSizeConstraints(const gfx::Size& min_size,
                                          const gfx::Size& max_size) = 0;
 
-  // Returns whether the window show be visible on all workspaces.
-  virtual void SetVisibleOnAllWorkspaces(bool always_visible) = 0;
-
   // Returns false if the underlying native window ignores alpha transparency
   // when compositing.
   virtual bool CanHaveAlphaEnabled() const = 0;

diff --git a/extensions/browser/extension_function_histogram_value.h b/extensions/browser/extension_function_histogram_value.h
@@ -948,7 +948,6 @@ enum HistogramValue {
   MEDIAGALLERIES_GETALLGALLERYWATCH,
   MEDIAGALLERIES_REMOVEALLGALLERYWATCH,
   MANAGEMENT_GETSELF,
-  APP_CURRENTWINDOWINTERNAL_SETVISIBLEONALLWORKSPACES,
   // Last entry: Add new entries above and ensure to update
   // tools/metrics/histograms/histograms.xml.
   ENUM_BOUNDARY

diff --git a/extensions/common/api/app_window.idl b/extensions/common/api/app_window.idl
@@ -263,10 +263,6 @@ namespace app.window {
 
     // If true, the window will be focused when created. Defaults to true.
     boolean? focused;
-
-    // If true, the window will be visible on all workspaces.
-    // This is only available on dev channel.
-    boolean? visibleOnAllWorkspaces;
   };
 
   // Called in the creating window (parent) before the load event is called in
@@ -373,11 +369,6 @@ namespace app.window {
     // TODO(jackhou): Document this properly before going to stable.
     [nodoc] static boolean alphaEnabled();
 
-    // For platforms that support multiple workspaces, is this window visible on
-    // all of them?
-    // This is only available on dev channel.
-    static void setVisibleOnAllWorkspaces(boolean alwaysVisible);
-
     // The JavaScript 'window' object for the created child.
     [instanceOf=Window] object contentWindow;
 
@@ -435,10 +426,6 @@ namespace app.window {
     // Gets an $(ref:AppWindow) with the given id. If no window with the given id
     // exists null is returned. This method is new in Chrome 33.
     [nocompile] static AppWindow get(DOMString id);
-
-    // Does the current platform support windows being visible on all
-    // workspaces?
-    [nocompile] static boolean canSetVisibleOnAllWorkspaces();
   };
 
   interface Events {

diff --git a/extensions/renderer/resources/app_window_custom_bindings.js b/extensions/renderer/resources/app_window_custom_bindings.js
@@ -193,10 +193,6 @@ appWindow.registerCustomHook(function(bindingsAPI) {
     return windows.length > 0 ? windows[0] : null;
   });
 
-  apiFunctions.setHandleRequest('canSetVisibleOnAllWorkspaces', function() {
-    return /Mac/.test(navigator.platform) || /Linux/.test(navigator.userAgent);
-  });
-
   // This is an internal function, but needs to be bound into a closure
   // so the correct JS context is used for global variables such as
   // currentWindowInternal, appWindowData, etc.

diff --git a/extensions/shell/browser/shell_native_app_window.cc b/extensions/shell/browser/shell_native_app_window.cc
@@ -238,10 +238,6 @@ void ShellNativeAppWindow::SetContentSizeConstraints(
   NOTIMPLEMENTED();
 }
 
-void ShellNativeAppWindow::SetVisibleOnAllWorkspaces(bool always_visible) {
-  NOTIMPLEMENTED();
-}
-
 bool ShellNativeAppWindow::CanHaveAlphaEnabled() const {
   NOTIMPLEMENTED();
   return false;

diff --git a/extensions/shell/browser/shell_native_app_window.h b/extensions/shell/browser/shell_native_app_window.h
@@ -75,7 +75,6 @@ class ShellNativeAppWindow : public NativeAppWindow {
   virtual gfx::Size GetContentMaximumSize() const OVERRIDE;
   virtual void SetContentSizeConstraints(const gfx::Size& min_size,
                                          const gfx::Size& max_size) OVERRIDE;
-  virtual void SetVisibleOnAllWorkspaces(bool always_visible) OVERRIDE;
   virtual bool CanHaveAlphaEnabled() const OVERRIDE;
 
  private:

diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
@@ -41408,7 +41408,6 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   <int value="887" label="MEDIAGALLERIES_GETALLGALLERYWATCH"/>
   <int value="888" label="MEDIAGALLERIES_REMOVEALLGALLERYWATCH"/>
   <int value="889" label="MANAGEMENT_GETSELF"/>
-  <int value="890" label="APP_CURRENTWINDOWINTERNAL_SETVISIBLEONALLWORKSPACES"/>
 </enum>
 
 <enum name="ExtensionInstallCause" type="int">