Re-enable 4 PDF accessibility tests by making them more robust.

Re-enable 4 PDF accessibility tests by making them more robust.

Work around issues where the PDF plug-in is returning inconsistent string
on different platforms, regarding whitespace and "fi" ligatures.

BUG=701427
Review-Url: https://codereview.chromium.org/2760053002
Cr-Commit-Position: refs/heads/master@{#458835}
Committed: https://chromium.googlesource.com/chromium/src/+/9d1abef69b6441eba82f137f2b38ef3d6935182a

[dmazzoni, 2017-03-20 18:03:59]

dmazzoni@chromium.org changed reviewers:
+ krasin@chromium.org, raymes@chromium.org, thestig@chromium.org

[krasin1, 2017-03-20 18:05:24]

LGTM

Thank you.

[dmazzoni, 2017-03-20 18:12:09]

The CQ bit was checked by dmazzoni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-20 18:12:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Lei Zhang, 2017-03-20 18:45:04]

lgtm

We can look at the bug separately to figure out why we are getting inconsistent
strings.

[commit-bot: I haz the power, 2017-03-20 19:27:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-20 19:27:44]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dmazzoni, 2017-03-21 21:37:35]

The CQ bit was checked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2017-03-21 21:38:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-21 23:27:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-21 23:27:26]

Failed to apply patch for chrome/browser/pdf/pdf_extension_test.cc:
While running git apply --index -p1;
  error: patch failed: chrome/browser/pdf/pdf_extension_test.cc:69
  error: chrome/browser/pdf/pdf_extension_test.cc: patch does not apply

Patch:       chrome/browser/pdf/pdf_extension_test.cc
Index: chrome/browser/pdf/pdf_extension_test.cc
diff --git a/chrome/browser/pdf/pdf_extension_test.cc
b/chrome/browser/pdf/pdf_extension_test.cc
index
3466cad9b60b8f2d50d617bdf323a022ad286c9b..0f1d33de763c569314829783bd801e0f55ddfa65
100644
--- a/chrome/browser/pdf/pdf_extension_test.cc
+++ b/chrome/browser/pdf/pdf_extension_test.cc
@@ -14,6 +14,7 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/path_service.h"
+#include "base/strings/pattern.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/browser/chrome_notification_types.h"
@@ -69,12 +70,13 @@ const int kDefaultKeyModifier =
blink::WebInputEvent::MetaKey;
 const int kDefaultKeyModifier = blink::WebInputEvent::ControlKey;
 #endif
 
-// Using ASSERT_TRUE deliberately instead of ASSERT_EQ or ASSERT_STREQ
-// in order to print a more readable message if the strings differ.
-#define ASSERT_MULTILINE_STREQ(expected, actual) \
-    ASSERT_TRUE(expected == actual) \
-        << "Expected:\n" << expected \
-        << "\n\nActual:\n" << actual
+// Check if the |actual| string matches the string or the string pattern in
+// |pattern| and print a readable message if it does not match.
+#define ASSERT_MULTILINE_STR_MATCHES(pattern, actual) \
+  ASSERT_TRUE(base::MatchPattern(actual, pattern))    \
+      << "Expected match pattern:\n"                  \
+      << pattern << "\n\nActual:\n"                   \
+      << actual
 
 bool GetGuestCallback(content::WebContents** guest_out,
                       content::WebContents* guest) {
@@ -603,8 +605,7 @@ static std::string DumpPdfAccessibilityTree(const
ui::AXTreeUpdate& ax_tree) {
     ax_tree_dump += ui::ToString(node.role);
 
     std::string name = node.GetStringAttribute(ui::AX_ATTR_NAME);
-    base::ReplaceChars(name, "\r", "\\r", &name);
-    base::ReplaceChars(name, "\n", "\\n", &name);
+    base::ReplaceChars(name, "\r\n", "", &name);
     if (!name.empty())
       ax_tree_dump += " '" + name + "'";
     ax_tree_dump += "\n";
@@ -615,38 +616,41 @@ static std::string DumpPdfAccessibilityTree(const
ui::AXTreeUpdate& ax_tree) {
   return ax_tree_dump;
 }
 
-static const char kExpectedPDFAXTree[] =
+// This is a pattern with a few wildcards due to a PDF bug where the
+// fi ligature is not parsed correctly on some systems.
+// http://crbug.com/701427
+
+static const char kExpectedPDFAXTreePattern[] =
     "embeddedObject\n"
     "  group\n"
     "    region 'Page 1'\n"
     "      paragraph\n"
-    "        staticText '1 First Section\\r\\n'\n"
+    "        staticText '1 First Section'\n"
     "          inlineTextBox '1 '\n"
-    "          inlineTextBox 'First Section\\r\\n'\n"
+    "          inlineTextBox 'First Section'\n"
     "      paragraph\n"
-    "        staticText 'This is the first section.\\r\\n1'\n"
-    "          inlineTextBox 'This is the first section.\\r\\n'\n"
+    "        staticText 'This is the *rst section.1'\n"
+    "          inlineTextBox 'This is the *rst section.'\n"
     "          inlineTextBox '1'\n"
     "    region 'Page 2'\n"
     "      paragraph\n"
-    "        staticText '1.1 First Subsection\\r\\n'\n"
+    "        staticText '1.1 First Subsection'\n"
     "          inlineTextBox '1.1 '\n"
-    "          inlineTextBox 'First Subsection\\r\\n'\n"
+    "          inlineTextBox 'First Subsection'\n"
     "      paragraph\n"
-    "        staticText 'This is the first subsection.\\r\\n2'\n"
-    "          inlineTextBox 'This is the first subsection.\\r\\n'\n"
+    "        staticText 'This is the *rst subsection.2'\n"
+    "          inlineTextBox 'This is the *rst subsection.'\n"
     "          inlineTextBox '2'\n"
     "    region 'Page 3'\n"
     "      paragraph\n"
-    "        staticText '2 Second Section\\r\\n'\n"
+    "        staticText '2 Second Section'\n"
     "          inlineTextBox '2 '\n"
-    "          inlineTextBox 'Second Section\\r\\n'\n"
+    "          inlineTextBox 'Second Section'\n"
     "      paragraph\n"
     "        staticText '3'\n"
     "          inlineTextBox '3'\n";
 
-// https://crbug.com/701427
-IN_PROC_BROWSER_TEST_F(PDFExtensionTest, DISABLED_PdfAccessibility) {
+IN_PROC_BROWSER_TEST_F(PDFExtensionTest, PdfAccessibility) {
   content::BrowserAccessibilityState::GetInstance()->EnableAccessibility();
 
   GURL test_pdf_url(embedded_test_server()->GetURL("/pdf/test-bookmarks.pdf"));
@@ -657,7 +661,8 @@ IN_PROC_BROWSER_TEST_F(PDFExtensionTest,
DISABLED_PdfAccessibility) {
                                                 "1 First Section\r\n");
   ui::AXTreeUpdate ax_tree = GetAccessibilityTreeSnapshot(guest_contents);
   std::string ax_tree_dump = DumpPdfAccessibilityTree(ax_tree);
-  ASSERT_MULTILINE_STREQ(kExpectedPDFAXTree, ax_tree_dump);
+
+  ASSERT_MULTILINE_STR_MATCHES(kExpectedPDFAXTreePattern, ax_tree_dump);
 }
 
 #if defined(GOOGLE_CHROME_BUILD)
@@ -675,8 +680,7 @@ IN_PROC_BROWSER_TEST_F(PDFExtensionTest,
PdfAccessibilityCharCountCrash) {
 }
 #endif
 
-// https://crbug.com/701427
-IN_PROC_BROWSER_TEST_F(PDFExtensionTest, DISABLED_PdfAccessibilityEnableLater)
{
+IN_PROC_BROWSER_TEST_F(PDFExtensionTest, PdfAccessibilityEnableLater) {
   // In this test, load the PDF file first, with accessibility off.
   GURL test_pdf_url(embedded_test_server()->GetURL("/pdf/test-bookmarks.pdf"));
   content::WebContents* guest_contents = LoadPdfGetGuestContents(test_pdf_url);
@@ -689,7 +693,7 @@ IN_PROC_BROWSER_TEST_F(PDFExtensionTest,
DISABLED_PdfAccessibilityEnableLater) {
                                                 "1 First Section\r\n");
   ui::AXTreeUpdate ax_tree = GetAccessibilityTreeSnapshot(guest_contents);
   std::string ax_tree_dump = DumpPdfAccessibilityTree(ax_tree);
-  ASSERT_MULTILINE_STREQ(kExpectedPDFAXTree, ax_tree_dump);
+  ASSERT_MULTILINE_STR_MATCHES(kExpectedPDFAXTreePattern, ax_tree_dump);
 }
 
 bool RetrieveGuestContents(
@@ -699,8 +703,7 @@ bool RetrieveGuestContents(
   return true;
 }
 
-// https://crbug.com/701427
-IN_PROC_BROWSER_TEST_F(PDFExtensionTest, DISABLED_PdfAccessibilityInIframe) {
+IN_PROC_BROWSER_TEST_F(PDFExtensionTest, PdfAccessibilityInIframe) {
   content::BrowserAccessibilityState::GetInstance()->EnableAccessibility();
   GURL
test_iframe_url(embedded_test_server()->GetURL("/pdf/test-iframe.html"));
   ui_test_utils::NavigateToURL(browser(), test_iframe_url);
@@ -719,11 +722,10 @@ IN_PROC_BROWSER_TEST_F(PDFExtensionTest,
DISABLED_PdfAccessibilityInIframe) {
 
   ui::AXTreeUpdate ax_tree = GetAccessibilityTreeSnapshot(guest_contents);
   std::string ax_tree_dump = DumpPdfAccessibilityTree(ax_tree);
-  ASSERT_MULTILINE_STREQ(kExpectedPDFAXTree, ax_tree_dump);
+  ASSERT_MULTILINE_STR_MATCHES(kExpectedPDFAXTreePattern, ax_tree_dump);
 }
 
-// https://crbug.com/701427
-IN_PROC_BROWSER_TEST_F(PDFExtensionTest, DISABLED_PdfAccessibilityInOOPIF) {
+IN_PROC_BROWSER_TEST_F(PDFExtensionTest, PdfAccessibilityInOOPIF) {
   content::BrowserAccessibilityState::GetInstance()->EnableAccessibility();
   GURL test_iframe_url(embedded_test_server()->GetURL(
       "/pdf/test-cross-site-iframe.html"));
@@ -743,7 +745,7 @@ IN_PROC_BROWSER_TEST_F(PDFExtensionTest,
DISABLED_PdfAccessibilityInOOPIF) {
 
   ui::AXTreeUpdate ax_tree = GetAccessibilityTreeSnapshot(guest_contents);
   std::string ax_tree_dump = DumpPdfAccessibilityTree(ax_tree);
-  ASSERT_MULTILINE_STREQ(kExpectedPDFAXTree, ax_tree_dump);
+  ASSERT_MULTILINE_STR_MATCHES(kExpectedPDFAXTreePattern, ax_tree_dump);
 }
 
 #if defined(GOOGLE_CHROME_BUILD)

[Lei Zhang, 2017-03-21 23:35:48]

On 2017/03/21 23:27:26, commit-bot: I haz the power wrote:
> Failed to apply patch for chrome/browser/pdf/pdf_extension_test.cc:

That's my fault for sneaking in another test ahead of this.

[dmazzoni, 2017-03-22 18:18:40]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2017-03-22 18:18:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, krasin@chromium.org
Link to the patchset: https://codereview.chromium.org/2760053002/#ps20001
(title: "Rebase")

[commit-bot: I haz the power, 2017-03-22 18:19:25]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-22 19:24:16]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490206720905470,
"parent_rev": "eb4d4be227e06523242c77c787e8155b3d7b39ee", "commit_rev":
"9d1abef69b6441eba82f137f2b38ef3d6935182a"}

[commit-bot: I haz the power, 2017-03-22 19:25:02]

Description was changed from

==========
Re-enable 4 PDF accessibility tests by making them more robust.

Work around issues where the PDF plug-in is returning inconsistent string
on different platforms, regarding whitespace and "fi" ligatures.

BUG=701427
==========

to

==========
Re-enable 4 PDF accessibility tests by making them more robust.

Work around issues where the PDF plug-in is returning inconsistent string
on different platforms, regarding whitespace and "fi" ligatures.

BUG=701427

Review-Url: https://codereview.chromium.org/2760053002
Cr-Commit-Position: refs/heads/master@{#458835}
Committed:
https://chromium.googlesource.com/chromium/src/+/9d1abef69b6441eba82f137f2b38...
==========

[commit-bot: I haz the power, 2017-03-22 19:25:07]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/9d1abef69b6441eba82f137f2b38...