From d118f54157b7a2cd0b76ff4c59027ae67024e438 Mon Sep 17 00:00:00 2001
From: "ananta@chromium.org"
 <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Fri, 5 Nov 2010 18:50:31 +0000
Subject: Back forward transitions would not work within ChromeFrame when
 attempted via the context menu. This was because the Back and Forward menu
 items on the context menu were disabled. The context menu is displayed by the
 active document in IE and it attempts to enable/disable these items based on
 entries in the IE travel log. The enabling of these items was failing as we
 were incorrectly passing in the string id of these options instead of their
 command id equivalents

Should fix bug http://code.google.com/p/chromium/issues/detail?id=34657

Bug=34657
Test=Covered by new ChromeFrame context menu back forward test.

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@65231 0039d316-1c4b-4281-b951-d872f2087c98
---
 chrome_frame/chrome_active_document.cc    | 11 ++---
 chrome_frame/test/test_with_web_server.cc |  9 +++-
 chrome_frame/test/test_with_web_server.h  |  6 +++
 chrome_frame/test/ui_test.cc              | 70 +++++++++++++++++++++++++++++++
 4 files changed, 90 insertions(+), 6 deletions(-)

(limited to 'chrome_frame')

diff --git a/chrome_frame/chrome_active_document.cc b/chrome_frame/chrome_active_document.cc
index 9dfe739..979d9d7 100644
--- a/chrome_frame/chrome_active_document.cc
+++ b/chrome_frame/chrome_active_document.cc
@@ -958,13 +958,14 @@ bool ChromeActiveDocument::PreProcessContextMenu(HMENU menu) {
   if (!browser_service || !travel_log)
     return true;
 
-  EnableMenuItem(menu, IDS_CONTENT_CONTEXT_BACK, MF_BYCOMMAND |
-      (SUCCEEDED(travel_log->GetTravelEntry(browser_service, TLOG_BACK, NULL)) ?
+  EnableMenuItem(menu, IDC_BACK, MF_BYCOMMAND |
+      (SUCCEEDED(travel_log->GetTravelEntry(browser_service, TLOG_BACK,
+                                            NULL)) ?
       MF_ENABLED : MF_DISABLED));
-  EnableMenuItem(menu, IDS_CONTENT_CONTEXT_FORWARD, MF_BYCOMMAND |
-      (SUCCEEDED(travel_log->GetTravelEntry(browser_service, TLOG_FORE, NULL)) ?
+  EnableMenuItem(menu, IDC_FORWARD, MF_BYCOMMAND |
+      (SUCCEEDED(travel_log->GetTravelEntry(browser_service, TLOG_FORE,
+                                            NULL)) ?
       MF_ENABLED : MF_DISABLED));
-
   // Call base class (adds 'About' item)
   return BaseActiveX::PreProcessContextMenu(menu);
 }
diff --git a/chrome_frame/test/test_with_web_server.cc b/chrome_frame/test/test_with_web_server.cc
index 05c931d..a03a46a 100644
--- a/chrome_frame/test/test_with_web_server.cc
+++ b/chrome_frame/test/test_with_web_server.cc
@@ -284,8 +284,15 @@ void ChromeFrameTestWithWebServer::VersionTest(BrowserKind browser,
 // MockWebServer methods
 void MockWebServer::ExpectAndServeRequest(CFInvocation invocation,
                                           const std::wstring& url) {
+  ExpectAndServeRequestWithCardinality(invocation, url, testing::Exactly(1));
+}
+
+void MockWebServer::ExpectAndServeRequestWithCardinality(
+    CFInvocation invocation, const std::wstring& url,
+    testing::Cardinality cardinality) {
   EXPECT_CALL(*this, Get(_, chrome_frame_test::UrlPathEq(url), _))
-      .WillOnce(SendResponse(this, invocation));
+      .Times(cardinality)
+      .WillRepeatedly(SendResponse(this, invocation));
 }
 
 void MockWebServer::ExpectAndServeRequestAllowCache(CFInvocation invocation,
diff --git a/chrome_frame/test/test_with_web_server.h b/chrome_frame/test/test_with_web_server.h
index 5c87791..fd65615 100644
--- a/chrome_frame/test/test_with_web_server.h
+++ b/chrome_frame/test/test_with_web_server.h
@@ -87,6 +87,12 @@ class MockWebServer : public test_server::HTTPTestServer {
   // is using the CF meta tag.
   void ExpectAndServeRequest(CFInvocation invocation, const std::wstring& url);
 
+  // Expect a number of GET requests for |url|. Rest is similar to the function
+  // ExpectAndServeRequest.
+  void ExpectAndServeRequestWithCardinality(CFInvocation invocation,
+                                            const std::wstring& url,
+                                            testing::Cardinality cardinality);
+
   // Same as above except do not include the no-cache header.
   void ExpectAndServeRequestAllowCache(CFInvocation invocation,
                                        const std::wstring& url);
diff --git a/chrome_frame/test/ui_test.cc b/chrome_frame/test/ui_test.cc
index 169bcc1..c308637 100644
--- a/chrome_frame/test/ui_test.cc
+++ b/chrome_frame/test/ui_test.cc
@@ -647,4 +647,74 @@ TEST_F(ContextMenuTest, IEBackForward) {
   LaunchIEAndNavigate(page1);
 }
 
+TEST_F(ContextMenuTest, CFBackForward) {
+  std::wstring page1 = GetLinkPageUrl();
+  std::wstring page2 = GetSimplePageUrl();
+  std::wstring page3 = GetTestUrl(L"anchor.html");
+
+  server_mock_.ExpectAndServeRequestWithCardinality(
+      CFInvocation::MetaTag(), page1, testing::Exactly(2));
+
+  server_mock_.ExpectAndServeRequestWithCardinality(
+      CFInvocation::None(), page2, testing::Exactly(3));
+
+  server_mock_.ExpectAndServeRequestWithCardinality(
+      CFInvocation::MetaTag(), page3, testing::Exactly(2));
+
+  InSequence expect_in_sequence_for_scope;
+
+  // Navigate to second page.
+  EXPECT_CALL(acc_observer_, OnAccDocLoad(_))
+      .WillOnce(testing::DoAll(
+          VerifyPageLoad(&ie_mock_, IN_CF, page1),
+          Navigate(&ie_mock_, page2)));
+
+  // Navigate to third page.
+  EXPECT_CALL(acc_observer_, OnAccDocLoad(_))
+      .WillOnce(testing::DoAll(
+          VerifyPageLoad(&ie_mock_, IN_IE, page2),
+          Navigate(&ie_mock_, page3)));
+
+  // Go back.
+  EXPECT_CALL(acc_observer_, OnAccDocLoad(_))
+      .WillOnce(testing::DoAll(
+          VerifyPageLoad(&ie_mock_, IN_CF, page3),
+          OpenContextMenuAsync()));
+
+  EXPECT_CALL(acc_observer_, OnMenuPopup(_))
+      .WillOnce(AccLeftClick(AccObjectMatcher(L"Back")));
+
+  // Go back
+  EXPECT_CALL(acc_observer_, OnAccDocLoad(_))
+      .WillOnce(testing::DoAll(
+          VerifyPageLoad(&ie_mock_, IN_IE, page2),
+          OpenContextMenuAsync()));
+
+  EXPECT_CALL(acc_observer_, OnMenuPopup(_))
+      .WillOnce(AccLeftClick(AccObjectMatcher(L"Back")));
+
+  // Go forward.
+  EXPECT_CALL(acc_observer_, OnAccDocLoad(_))
+      .WillOnce(testing::DoAll(
+          VerifyPageLoad(&ie_mock_, IN_CF, page1),
+          OpenContextMenuAsync()));
+
+  EXPECT_CALL(acc_observer_, OnMenuPopup(_))
+      .WillOnce(AccLeftClick(AccObjectMatcher(L"Forward")));
+
+  // Go forward.
+  EXPECT_CALL(acc_observer_, OnAccDocLoad(_))
+      .WillOnce(testing::DoAll(
+          VerifyPageLoad(&ie_mock_, IN_IE, page2),
+          OpenContextMenuAsync()));
+
+  EXPECT_CALL(acc_observer_, OnMenuPopup(_))
+      .WillOnce(AccLeftClick(AccObjectMatcher(L"Forward")));
+
+  EXPECT_CALL(ie_mock_, OnLoad(IN_CF, StrEq(page3)))
+      .WillOnce(CloseBrowserMock(&ie_mock_));
+
+  LaunchIEAndNavigate(page1);
+}
+
 }  // namespace chrome_frame_test
-- 
cgit v1.1