Commit cfb21748 authored by sail@chromium.org's avatar sail@chromium.org

Revert 170795

The ConstrainedWindowMacTest.ShowInUninitializedTab was failing on some bots.
http://build.chromium.org/p/tryserver.chromium/builders/mac_rel/builds/78878/steps/browser_tests/logs/stdio

> Fix showing constrained window for uninitialized tabs
> 
> If a tab is added without being shown then the associated tab view for the web content will not be created yet.
> 
> When a constrained window is added to that tab the window never gets shown.
> 
> To fix this I've changed ConstrainedWindowMac2 to reshow the window once the web contents becomes visible.
> 
> BUG=163778
> TEST=Reproduced bug 163778 and verified that my patch fixes it.
> Verified that ConstrainedWindowMacTest.ShowInUninitializedTab fails without my patch and passes with it.
> 
> 
> Review URL: https://chromiumcodereview.appspot.com/11419277

TBR=sail@chromium.org
Review URL: https://codereview.chromium.org/11418304

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@170903 0039d316-1c4b-4281-b951-d872f2087c98
parent aa19063b
...@@ -9,8 +9,6 @@ ...@@ -9,8 +9,6 @@
#include "base/memory/scoped_nsobject.h" #include "base/memory/scoped_nsobject.h"
#include "chrome/browser/ui/constrained_window.h" #include "chrome/browser/ui/constrained_window.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
namespace content { namespace content {
class WebContents; class WebContents;
...@@ -27,8 +25,7 @@ class ConstrainedWindowMacDelegate2 { ...@@ -27,8 +25,7 @@ class ConstrainedWindowMacDelegate2 {
// Constrained window implementation for Mac. // Constrained window implementation for Mac.
// Normally an instance of this class is owned by the delegate. The delegate // Normally an instance of this class is owned by the delegate. The delegate
// should delete the instance when the window is closed. // should delete the instance when the window is closed.
class ConstrainedWindowMac2 : public ConstrainedWindow, class ConstrainedWindowMac2 : public ConstrainedWindow {
public content::NotificationObserver {
public: public:
ConstrainedWindowMac2(ConstrainedWindowMacDelegate2* delegate, ConstrainedWindowMac2(ConstrainedWindowMacDelegate2* delegate,
content::WebContents* web_contents, content::WebContents* web_contents,
...@@ -43,11 +40,6 @@ class ConstrainedWindowMac2 : public ConstrainedWindow, ...@@ -43,11 +40,6 @@ class ConstrainedWindowMac2 : public ConstrainedWindow,
virtual gfx::NativeWindow GetNativeWindow() OVERRIDE; virtual gfx::NativeWindow GetNativeWindow() OVERRIDE;
virtual bool CanShowConstrainedWindow() OVERRIDE; virtual bool CanShowConstrainedWindow() OVERRIDE;
// content::NotificationObserver:
virtual void Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) OVERRIDE;
private: private:
// Gets the parent window of the dialog. // Gets the parent window of the dialog.
NSWindow* GetParentWindow() const; NSWindow* GetParentWindow() const;
...@@ -58,12 +50,6 @@ class ConstrainedWindowMac2 : public ConstrainedWindow, ...@@ -58,12 +50,6 @@ class ConstrainedWindowMac2 : public ConstrainedWindow,
content::WebContents* web_contents_; content::WebContents* web_contents_;
scoped_nsobject<NSWindow> window_; scoped_nsobject<NSWindow> window_;
// A scoped container for notification registries.
content::NotificationRegistrar registrar_;
// This is true if the constrained window is waiting to be shown.
bool pending_show_;
}; };
#endif // CHROME_BROWSER_UI_COCOA_CONSTRAINED_WINDOW_CONSTRAINED_WINDOW_MAC_2_ #endif // CHROME_BROWSER_UI_COCOA_CONSTRAINED_WINDOW_CONSTRAINED_WINDOW_MAC_2_
...@@ -10,8 +10,6 @@ ...@@ -10,8 +10,6 @@
#include "chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.h" #include "chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.h"
#include "chrome/browser/ui/cocoa/tabs/tab_strip_controller.h" #include "chrome/browser/ui/cocoa/tabs/tab_strip_controller.h"
#include "chrome/browser/ui/constrained_window_tab_helper.h" #include "chrome/browser/ui/constrained_window_tab_helper.h"
#include "content/public/browser/notification_source.h"
#include "content/public/browser/notification_types.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_view.h" #include "content/public/browser/web_contents_view.h"
...@@ -21,17 +19,12 @@ ConstrainedWindowMac2::ConstrainedWindowMac2( ...@@ -21,17 +19,12 @@ ConstrainedWindowMac2::ConstrainedWindowMac2(
NSWindow* window) NSWindow* window)
: delegate_(delegate), : delegate_(delegate),
web_contents_(web_contents), web_contents_(web_contents),
window_([window retain]), window_([window retain]) {
pending_show_(false) {
DCHECK(web_contents); DCHECK(web_contents);
DCHECK(window_.get()); DCHECK(window_.get());
ConstrainedWindowTabHelper* constrained_window_tab_helper = ConstrainedWindowTabHelper* constrained_window_tab_helper =
ConstrainedWindowTabHelper::FromWebContents(web_contents); ConstrainedWindowTabHelper::FromWebContents(web_contents);
constrained_window_tab_helper->AddConstrainedDialog(this); constrained_window_tab_helper->AddConstrainedDialog(this);
registrar_.Add(this,
content::NOTIFICATION_WEB_CONTENTS_VISIBILITY_CHANGED,
content::Source<content::WebContents>(web_contents));
} }
ConstrainedWindowMac2::~ConstrainedWindowMac2() { ConstrainedWindowMac2::~ConstrainedWindowMac2() {
...@@ -39,11 +32,11 @@ ConstrainedWindowMac2::~ConstrainedWindowMac2() { ...@@ -39,11 +32,11 @@ ConstrainedWindowMac2::~ConstrainedWindowMac2() {
void ConstrainedWindowMac2::ShowConstrainedWindow() { void ConstrainedWindowMac2::ShowConstrainedWindow() {
NSWindow* parent_window = GetParentWindow(); NSWindow* parent_window = GetParentWindow();
NSView* parent_view = GetSheetParentViewForWebContents(web_contents_); if (!parent_window)
if (!parent_window || !parent_view) {
pending_show_ = true;
return; return;
}
NSView* parent_view = GetSheetParentViewForWebContents(web_contents_);
DCHECK(parent_view);
ConstrainedWindowSheetController* controller = ConstrainedWindowSheetController* controller =
[ConstrainedWindowSheetController [ConstrainedWindowSheetController
...@@ -52,10 +45,6 @@ void ConstrainedWindowMac2::ShowConstrainedWindow() { ...@@ -52,10 +45,6 @@ void ConstrainedWindowMac2::ShowConstrainedWindow() {
} }
void ConstrainedWindowMac2::CloseConstrainedWindow() { void ConstrainedWindowMac2::CloseConstrainedWindow() {
// This function may be called even if the constrained window was never shown.
// Unset |pending_show_| to prevent the window from being reshown.
pending_show_ = false;
[[ConstrainedWindowSheetController controllerForSheet:window_] [[ConstrainedWindowSheetController controllerForSheet:window_]
closeSheet:window_]; closeSheet:window_];
ConstrainedWindowTabHelper* constrained_window_tab_helper = ConstrainedWindowTabHelper* constrained_window_tab_helper =
...@@ -81,21 +70,6 @@ bool ConstrainedWindowMac2::CanShowConstrainedWindow() { ...@@ -81,21 +70,6 @@ bool ConstrainedWindowMac2::CanShowConstrainedWindow() {
return !browser->window()->IsInstantTabShowing(); return !browser->window()->IsInstantTabShowing();
} }
void ConstrainedWindowMac2::Observe(
int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) {
if (type != content::NOTIFICATION_WEB_CONTENTS_VISIBILITY_CHANGED) {
NOTREACHED();
return;
}
if (pending_show_) {
pending_show_ = false;
ShowConstrainedWindow();
}
}
NSWindow* ConstrainedWindowMac2::GetParentWindow() const { NSWindow* ConstrainedWindowMac2::GetParentWindow() const {
// Tab contents in a tabbed browser may not be inside a window. For this // Tab contents in a tabbed browser may not be inside a window. For this
// reason use a browser window if possible. // reason use a browser window if possible.
......
...@@ -5,10 +5,8 @@ ...@@ -5,10 +5,8 @@
#include "chrome/browser/ui/cocoa/constrained_window/constrained_window_mac2.h" #include "chrome/browser/ui/cocoa/constrained_window/constrained_window_mac2.h"
#include "chrome/app/chrome_command_ids.h" #include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_commands.h" #include "chrome/browser/ui/browser_commands.h"
#include "chrome/browser/ui/browser_tabstrip.h"
#include "chrome/browser/ui/browser_window.h" #include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/cocoa/browser_window_controller.h" #include "chrome/browser/ui/cocoa/browser_window_controller.h"
#include "chrome/browser/ui/tab_contents/tab_contents.h" #include "chrome/browser/ui/tab_contents/tab_contents.h"
...@@ -16,7 +14,6 @@ ...@@ -16,7 +14,6 @@
#include "chrome/test/base/in_process_browser_test.h" #include "chrome/test/base/in_process_browser_test.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "googleurl/src/gurl.h" #include "googleurl/src/gurl.h"
#include "ipc/ipc_message.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
using ::testing::NiceMock; using ::testing::NiceMock;
...@@ -86,34 +83,6 @@ IN_PROC_BROWSER_TEST_F(ConstrainedWindowMacTest, ShowInInactiveTab) { ...@@ -86,34 +83,6 @@ IN_PROC_BROWSER_TEST_F(ConstrainedWindowMacTest, ShowInInactiveTab) {
dialog.CloseConstrainedWindow(); dialog.CloseConstrainedWindow();
} }
// If a tab has never been shown then the associated tab view for the web
// content will not be created. Verify that adding a constrained window to such
// a tab works correctly.
IN_PROC_BROWSER_TEST_F(ConstrainedWindowMacTest, ShowInUninitializedTab) {
scoped_ptr<content::WebContents> web_contents(content::WebContents::Create(
browser()->profile(), NULL, MSG_ROUTING_NONE, NULL));
bool was_blocked = false;
chrome::AddWebContents(browser(), NULL, web_contents.release(),
NEW_BACKGROUND_TAB, gfx::Rect(), false, &was_blocked);
content::WebContents* tab2 =
browser()->tab_strip_model()->GetWebContentsAt(2);
ASSERT_TRUE(tab2);
EXPECT_FALSE([tab2->GetNativeView() superview]);
// Show dialog and verify that it's not visible yet.
NiceMock<ConstrainedWindowDelegateMock> delegate;
ConstrainedWindowMac2 dialog(&delegate, tab2, sheet_);
EXPECT_FALSE([sheet_ isVisible]);
// Activate the tab and verify that the constrained window is shown.
browser()->tab_strip_model()->ActivateTabAt(2, true);
EXPECT_TRUE([tab2->GetNativeView() superview]);
EXPECT_TRUE([sheet_ isVisible]);
EXPECT_EQ(1.0, [sheet_ alphaValue]);
dialog.CloseConstrainedWindow();
}
// Test that adding a sheet disables tab dragging. // Test that adding a sheet disables tab dragging.
IN_PROC_BROWSER_TEST_F(ConstrainedWindowMacTest, TabDragging) { IN_PROC_BROWSER_TEST_F(ConstrainedWindowMacTest, TabDragging) {
NiceMock<ConstrainedWindowDelegateMock> delegate; NiceMock<ConstrainedWindowDelegateMock> delegate;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment