Skip to content

Commit

Permalink
Remove usage of the old page info dialogs in ChromeFrame and use the …
Browse files Browse the repository at this point in the history
…new page info bubble

instead. Changed the expectations of the corresponding chrome frame ui test to look for
this window.

Added support in ChromeFrame tests window watcher class to allow specifying window class names
as well.

Fixes bug http://code.google.com/p/chromium/issues/detail?id=59030

Bug=59030

Review URL: http://codereview.chromium.org/3864001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@63138 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
ananta@chromium.org committed Oct 19, 2010
1 parent 1af1dfe commit 0b7a012
Show file tree
Hide file tree
Showing 10 changed files with 102 additions and 27 deletions.
45 changes: 44 additions & 1 deletion chrome/browser/external_tab_container_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <string>

#include "app/l10n_util.h"
#include "base/i18n/rtl.h"
#include "base/logging.h"
#include "base/trace_event.h"
Expand All @@ -26,6 +27,7 @@
#include "chrome/browser/tab_contents/provisional_load_details.h"
#include "chrome/browser/views/tab_contents/render_view_context_menu_views.h"
#include "chrome/browser/tab_contents/tab_contents.h"
#include "chrome/browser/views/page_info_bubble_view.h"
#include "chrome/browser/views/tab_contents/tab_contents_container.h"
#include "chrome/common/bindings_policy.h"
#include "chrome/common/chrome_constants.h"
Expand All @@ -36,12 +38,40 @@
#include "chrome/common/page_transition_types.h"
#include "chrome/test/automation/automation_messages.h"
#include "grit/generated_resources.h"
#include "grit/locale_settings.h"
#include "views/grid_layout.h"
#include "views/widget/root_view.h"
#include "views/window/window.h"

static const wchar_t kWindowObjectKey[] = L"ChromeWindowObject";

// This class overrides the LinkActivated function in the PageInfoBubbleView
// class and routes the help center link navigation to the host browser.
class ExternalTabPageInfoBubbleView : public PageInfoBubbleView {
public:
ExternalTabPageInfoBubbleView(ExternalTabContainer* container,
gfx::NativeWindow parent_window,
Profile* profile,
const GURL& url,
const NavigationEntry::SSLStatus& ssl,
bool show_history)
: PageInfoBubbleView(parent_window, profile, url, ssl, show_history),
container_(container) {
DLOG(INFO) << __FUNCTION__;
}
virtual ~ExternalTabPageInfoBubbleView() {
DLOG(INFO) << __FUNCTION__;
}
// LinkController methods:
virtual void LinkActivated(views::Link* source, int event_flags) {
GURL url = GURL(l10n_util::GetStringUTF16(IDS_PAGE_INFO_HELP_CENTER));
container_->OpenURLFromTab(container_->tab_contents(), url, GURL(),
NEW_FOREGROUND_TAB, PageTransition::LINK);
}
private:
scoped_refptr<ExternalTabContainer> container_;
};

base::LazyInstance<ExternalTabContainer::PendingTabs>
ExternalTabContainer::pending_tabs_(base::LINKER_INITIALIZED);

Expand Down Expand Up @@ -524,7 +554,20 @@ void ExternalTabContainer::ShowPageInfo(Profile* profile,
const GURL& url,
const NavigationEntry::SSLStatus& ssl,
bool show_history) {
browser::ShowPageInfo(GetNativeView(), profile, url, ssl, show_history);
POINT cursor_pos = {0};
GetCursorPos(&cursor_pos);

gfx::Rect bounds;
bounds.set_origin(gfx::Point(cursor_pos));

PageInfoBubbleView* page_info_bubble =
new ExternalTabPageInfoBubbleView(this, NULL, profile, url,
ssl, show_history);
InfoBubble* info_bubble =
InfoBubble::Show(this, bounds,
BubbleBorder::TOP_LEFT,
page_info_bubble, page_info_bubble);
page_info_bubble->set_info_bubble(info_bubble);
}

void ExternalTabContainer::RegisterRenderViewHostForAutomation(
Expand Down
2 changes: 1 addition & 1 deletion chrome_frame/test/delete_chrome_history_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ TEST_F(DeleteBrowsingHistoryTest, DISABLED_CFDeleteBrowsingHistory) {
PageLoadHelper load_helper3(&ie_mock3_);

delete_browsing_history_window_observer_mock_.WatchWindow(
"Delete Browsing History");
"Delete Browsing History", "");

// For some reason, this page is occasionally being cached, so we randomize
// its name to ensure that, at least the first time we request it, it is
Expand Down
6 changes: 3 additions & 3 deletions chrome_frame/test/mock_ie_event_sink_actions.h
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,8 @@ ACTION_P2(PostCharMessagesToRenderer, mock, character_codes) {
::PostMessage(window, WM_CHAR, codes[i], 0);
}

ACTION_P2(WatchWindow, mock, window_class) {
mock->WatchWindow(window_class);
ACTION_P3(WatchWindow, mock, caption, window_class) {
mock->WatchWindow(caption, window_class);
}

ACTION_P(StopWindowWatching, mock) {
Expand All @@ -244,7 +244,7 @@ ACTION_P2(WatchRendererProcess, mock_observer, mock) {
namespace { // NOLINT

void DoCloseWindowNow(HWND hwnd) {
::PostMessage(hwnd, WM_SYSCOMMAND, SC_CLOSE, 0);
::PostMessage(hwnd, WM_CLOSE, 0, 0);
}

} // namespace
Expand Down
4 changes: 2 additions & 2 deletions chrome_frame/test/mock_ie_event_sink_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,8 @@ class MockWindowObserver : public WindowObserver {
MOCK_METHOD1(OnWindowOpen, void (HWND hwnd)); // NOLINT
MOCK_METHOD1(OnWindowClose, void (HWND hwnd)); // NOLINT

void WatchWindow(std::string caption_pattern) {
window_watcher_.AddObserver(this, caption_pattern);
void WatchWindow(std::string caption_pattern, std::string class_pattern) {
window_watcher_.AddObserver(this, caption_pattern, class_pattern);
}

private:
Expand Down
6 changes: 3 additions & 3 deletions chrome_frame/test/navigation_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ TEST_P(FullTabNavigationTest, FLAKY_RestrictedSite) {
testing::Field(&VARIANT::bstrVal, testing::StrCaseEq(GetSimplePageUrl())),
_, _, _, _, _))
.Times(1)
.WillOnce(WatchWindow(&win_observer_mock, kAlertDlgCaption));
.WillOnce(WatchWindow(&win_observer_mock, kAlertDlgCaption, ""));

if (patch_method == PATCH_METHOD_INET_PROTOCOL) {
EXPECT_CALL(ie_mock_, OnBeforeNavigate2(
Expand Down Expand Up @@ -742,10 +742,10 @@ void CloseWindow(HWND* window) {
// and validate that all is well.
TEST_F(FullTabDownloadTest, CF_DownloadFileFromPost) {
chrome_frame_test::MockWindowObserver download_watcher;
download_watcher.WatchWindow("File Download");
download_watcher.WatchWindow("File Download", "");

chrome_frame_test::MockWindowObserver save_dialog_watcher;
save_dialog_watcher.WatchWindow("Save As");
save_dialog_watcher.WatchWindow("Save As", "");

EXPECT_CALL(server_mock_, Get(_, StrEq(L"/post_source.html"), _)).WillOnce(
SendFast(
Expand Down
4 changes: 3 additions & 1 deletion chrome_frame/test/net/fake_external_tab.cc
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ void CFUrlRequestUnittestRunner::Initialize() {
void CFUrlRequestUnittestRunner::Shutdown() {
DCHECK(::GetCurrentThreadId() == test_thread_id_);
NetTestSuite::Shutdown();
OleUninitialize();
}

void CFUrlRequestUnittestRunner::OnConnectAutomationProviderToChannel(
Expand All @@ -342,6 +343,7 @@ void CFUrlRequestUnittestRunner::OnInitialTabLoaded() {
void CFUrlRequestUnittestRunner::RunMainUIThread() {
DCHECK(MessageLoop::current());
DCHECK(MessageLoop::current()->type() == MessageLoop::TYPE_UI);
OleInitialize(NULL);
MessageLoop::current()->Run();
}

Expand Down Expand Up @@ -473,7 +475,7 @@ int main(int argc, char** argv) {
WindowWatchdog watchdog;
// See url_request_unittest.cc for these credentials.
SupplyProxyCredentials credentials("user", "secret");
watchdog.AddObserver(&credentials, "Windows Security");
watchdog.AddObserver(&credentials, "Windows Security", "");
testing::InitGoogleTest(&argc, argv);
FilterDisabledTests();
PluginService::EnableChromePlugins(false);
Expand Down
2 changes: 1 addition & 1 deletion chrome_frame/test/test_with_web_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1120,7 +1120,7 @@ TEST_F(ChromeFrameTestWithWebServer, FAILS_FullTabModeIE_RefreshMshtmlTest) {
// See bug 36694 for details. http://crbug.com/36694
TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_TestDownloadFromForm) {
chrome_frame_test::MockWindowObserver win_observer_mock;
win_observer_mock.WatchWindow("File Download");
win_observer_mock.WatchWindow("File Download", "");

// The content of our HTML test page. This will be returned whenever
// we reply to a GET request.
Expand Down
13 changes: 7 additions & 6 deletions chrome_frame/test/ui_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ TEST_P(FullTabUITest, FLAKY_CtrlN) {
const char* kNewWindowTitlePattern = "*Internet Explorer*";
EXPECT_CALL(ie_mock_, OnLoad(is_cf, StrEq(GetSimplePageUrl())))
.WillOnce(testing::DoAll(
WatchWindow(&win_observer_mock, kNewWindowTitlePattern),
WatchWindow(&win_observer_mock, kNewWindowTitlePattern, ""),
SetFocusToRenderer(&ie_mock_),
DelaySendChar(&loop_, 1000, 'n', simulate_input::CONTROL)));

Expand Down Expand Up @@ -145,7 +145,7 @@ TEST_P(FullTabUITest, FLAKY_CtrlF) {
const char* kFindDialogCaption = "Find";
EXPECT_CALL(ie_mock_, OnLoad(IN_CF, StrEq(GetSimplePageUrl())))
.WillOnce(testing::DoAll(
WatchWindow(&win_observer_mock, kFindDialogCaption),
WatchWindow(&win_observer_mock, kFindDialogCaption, ""),
SetFocusToRenderer(&ie_mock_),
DelaySendChar(&loop_, 1500, 'f', simulate_input::CONTROL)));

Expand Down Expand Up @@ -366,7 +366,7 @@ class ContextMenuTest : public MockIEEventSinkTest, public testing::Test {
const char* kSaveDlgCaption = "Save As";
EXPECT_CALL(acc_observer_, OnAccDocLoad(_))
.WillOnce(testing::DoAll(
WatchWindow(&win_observer_mock, kSaveDlgCaption),
WatchWindow(&win_observer_mock, kSaveDlgCaption, ""),
AccRightClick(AccObjectMatcher(L"", role))));
EXPECT_CALL(acc_observer_, OnMenuPopup(_))
.WillOnce(AccLeftClick(AccObjectMatcher(menu_item_name)));
Expand Down Expand Up @@ -454,18 +454,19 @@ TEST_F(ContextMenuTest, CFPageInfo) {
InSequence expect_in_sequence_for_scope;

// View page information.
const char* kPageInfoCaption = "Security Information";
EXPECT_CALL(acc_observer_, OnAccDocLoad(_))
.WillOnce(testing::DoAll(
WatchWindow(&win_observer_mock, kPageInfoCaption),
WatchWindow(&win_observer_mock, "", "Chrome_WidgetWin_*"),
OpenContextMenuAsync()));
EXPECT_CALL(acc_observer_, OnMenuPopup(_))
.WillOnce(AccLeftClick(AccObjectMatcher(L"View page info")));

EXPECT_CALL(win_observer_mock, OnWindowOpen(_)).Times(1);
// Expect page info dialog to pop up. Dismiss the dialog with 'Esc' key
EXPECT_CALL(win_observer_mock, OnWindowOpen(_))
.WillOnce(DoCloseWindow());

EXPECT_CALL(win_observer_mock, OnWindowClose(_)).Times(1);
EXPECT_CALL(win_observer_mock, OnWindowClose(_))
.WillOnce(CloseBrowserMock(&ie_mock_));

Expand All @@ -483,7 +484,7 @@ TEST_F(ContextMenuTest, CFInspector) {
const char* kPageInfoCaptionPattern = "Untitled*";
EXPECT_CALL(acc_observer_, OnAccDocLoad(_))
.WillOnce(testing::DoAll(
WatchWindow(&win_observer_mock, kPageInfoCaptionPattern),
WatchWindow(&win_observer_mock, kPageInfoCaptionPattern, ""),
OpenContextMenuAsync()));
EXPECT_CALL(acc_observer_, OnMenuPopup(_))
.WillOnce(AccLeftClick(AccObjectMatcher(L"Inspect element")));
Expand Down
29 changes: 24 additions & 5 deletions chrome_frame/test/win_event_receiver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ void WindowWatchdog::ProcessExitObserver::OnObjectSignaled(
WindowWatchdog::WindowWatchdog() {}

void WindowWatchdog::AddObserver(WindowObserver* observer,
const std::string& caption_pattern) {
const std::string& caption_pattern,
const std::string& class_name_pattern) {
if (observers_.empty()) {
// SetListenerForEvents takes an event_min and event_max.
// EVENT_OBJECT_DESTROY, EVENT_OBJECT_SHOW, and EVENT_OBJECT_HIDE are
Expand All @@ -149,6 +150,7 @@ void WindowWatchdog::AddObserver(WindowObserver* observer,
ObserverEntry new_entry = {
observer,
caption_pattern,
class_name_pattern,
OpenWindowList() };

observers_.push_back(new_entry);
Expand All @@ -172,8 +174,28 @@ std::string WindowWatchdog::GetWindowCaption(HWND hwnd) {
return caption;
}

bool WindowWatchdog::MatchingWindow(const ObserverEntry& entry,
const std::string& caption,
const std::string& class_name) {
bool should_match_caption = !entry.caption_pattern.empty();
bool should_match_class = !entry.class_name_pattern.empty();

if (should_match_caption &&
MatchPattern(caption, entry.caption_pattern) &&
!should_match_class) {
return true;
}
if (should_match_class &&
MatchPattern(class_name, entry.class_name_pattern)) {
return true;
}
return false;
}

void WindowWatchdog::HandleOnOpen(HWND hwnd) {
std::string caption = GetWindowCaption(hwnd);
char class_name[MAX_PATH] = {0};
GetClassNameA(hwnd, class_name, arraysize(class_name));

// Instantiated only if there is at least one interested observer. Each
// interested observer will maintain a reference to this object, such that it
Expand All @@ -185,7 +207,7 @@ void WindowWatchdog::HandleOnOpen(HWND hwnd) {
ObserverEntryList interested_observers;
for (ObserverEntryList::iterator entry_iter = observers_.begin();
entry_iter != observers_.end(); ++entry_iter) {
if (MatchPattern(caption, entry_iter->caption_pattern)) {
if (MatchingWindow(*entry_iter, caption, class_name)) {
if (process_exit_observer == NULL) {
process_exit_observer.reset(new ProcessExitObserver(this, hwnd));
}
Expand Down Expand Up @@ -240,9 +262,6 @@ void WindowWatchdog::OnEventReceived(
// We need to look for top level windows and a natural check is for
// WS_CHILD. Instead, checking for WS_CAPTION allows us to filter
// out other stray popups
if (!WS_CAPTION & GetWindowLong(hwnd, GWL_STYLE)) {
return;
}
if (event == EVENT_OBJECT_SHOW) {
HandleOnOpen(hwnd);
} else {
Expand Down
18 changes: 14 additions & 4 deletions chrome_frame/test/win_event_receiver.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,14 @@ class WindowWatchdog : public WinEventListener {
public:
WindowWatchdog();
// Register |observer| to be notified when windows matching |caption_pattern|
// are opened or closed. A single observer may be registered multiple times.
// If a single window caption matches multiple registrations of a single
// observer, the observer will be notified once per matching registration.
// and/or |class_name_pattern| are opened or closed. A single observer may be
// registered multiple times.
// If a single window caption and/or class name matches multiple
// registrations of a single observer, the observer will be notified once per
// matching registration.
void AddObserver(WindowObserver* observer,
const std::string& caption_pattern);
const std::string& caption_pattern,
const std::string& class_name_pattern);

// Remove all registrations of |observer|. The |observer| will not be notified
// during or after this call.
Expand All @@ -110,6 +113,7 @@ class WindowWatchdog : public WinEventListener {
struct ObserverEntry {
WindowObserver* observer;
std::string caption_pattern;
std::string class_name_pattern;
OpenWindowList open_windows;
};

Expand All @@ -125,6 +129,12 @@ class WindowWatchdog : public WinEventListener {
void HandleOnClose(HWND hwnd);
void OnHwndProcessExited(HWND hwnd);

// Returns true if the caption pattern and/or the class name pattern in the
// observer entry structure matches the caption and/or class name passed in.
bool MatchingWindow(const ObserverEntry& entry,
const std::string& caption,
const std::string& class_name);

ObserverEntryList observers_;
WinEventReceiver win_event_receiver_;

Expand Down

0 comments on commit 0b7a012

Please sign in to comment.