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

4 files changed, 90 insertions, 6 deletions

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