chrome/browser/ui/find_bar/find_bar_host_browsertest.cc - Issue 1855393007: MD Downloads: ensure the DOM is ready before trying to find-in-page on it

Side by Side Diff: chrome/browser/ui/find_bar/find_bar_host_browsertest.cc

Issue 1855393007: MD Downloads: ensure the DOM is ready before trying to find-in-page on it (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Created 4 years, 8 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« no previous file with comments | « no previous file | no next file » | no next file with comments »

OLD	NEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.	1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.

2 // Use of this source code is governed by a BSD-style license that can be	2 // Use of this source code is governed by a BSD-style license that can be

3 // found in the LICENSE file.	3 // found in the LICENSE file.

4	4

5 #include <stddef.h>	5 #include <stddef.h>

6	6

7 #include "base/command_line.h"	7 #include "base/command_line.h"

8 #include "base/files/file_util.h"	8 #include "base/files/file_util.h"

9 #include "base/message_loop/message_loop.h"	9 #include "base/message_loop/message_loop.h"

10 #include "base/strings/string16.h"	10 #include "base/strings/string16.h"

(...skipping 318 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
329 ui_test_utils::FindInPage(web_contents, query,	329 ui_test_utils::FindInPage(web_contents, query,

330 kFwd, kIgnoreCase, NULL, NULL));	330 kFwd, kIgnoreCase, NULL, NULL));

331	331

332 GURL download_url = ui_test_utils::GetTestUrl(	332 GURL download_url = ui_test_utils::GetTestUrl(

333 base::FilePath().AppendASCII("downloads"),	333 base::FilePath().AppendASCII("downloads"),

334 base::FilePath().AppendASCII("a_zip_file.zip"));	334 base::FilePath().AppendASCII("a_zip_file.zip"));

335 ui_test_utils::DownloadURL(browser(), download_url);	335 ui_test_utils::DownloadURL(browser(), download_url);

336	336

337 ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUIDownloadsURL));	337 ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUIDownloadsURL));

338 FlushHistoryService();	338 FlushHistoryService();

	339 ASSERT_TRUE(content::ExecuteScript(web_contents, "Polymer.dom.flush();"));
	dpapad 2016/04/07 00:59:55 Nit(optional): Perhaps adding a comment to the eff Nit(optional): Perhaps adding a comment to the effect of "Force some async operations to happen synchronously" would help future readers. Dan Beam 2016/04/07 01:02:35 well, it's named Polymer.dom.flush(), i.e. flush P Show quoted text On 2016/04/07 00:59:55, dpapad wrote: > Nit(optional): Perhaps adding a comment to the effect of > "Force some async operations to happen synchronously" would help future readers. well, it's named Polymer.dom.flush(), i.e. flush Polymer-based DOM. also, just googling is probably better / more future proof. sky 2016/04/07 19:11:38 Actually, one question. Why isn't ui_test_utils::N Show quoted text On 2016/04/07 01:02:35, Dan Beam wrote: > On 2016/04/07 00:59:55, dpapad wrote: > > Nit(optional): Perhaps adding a comment to the effect of > > "Force some async operations to happen synchronously" would help future > readers. > > well, it's named Polymer.dom.flush(), i.e. flush Polymer-based DOM. also, just > googling is probably better / more future proof. Actually, one question. Why isn't ui_test_utils::NavigateToURL enough? Dan Beam 2016/04/07 19:12:21 because the list is rendered after onload Show quoted text On 2016/04/07 19:11:38, sky wrote: > On 2016/04/07 01:02:35, Dan Beam wrote: > > On 2016/04/07 00:59:55, dpapad wrote: > > > Nit(optional): Perhaps adding a comment to the effect of > > > "Force some async operations to happen synchronously" would help future > > readers. > > > > well, it's named Polymer.dom.flush(), i.e. flush Polymer-based DOM. also, > just > > googling is probably better / more future proof. > > Actually, one question. Why isn't ui_test_utils::NavigateToURL enough? because the list is rendered after onload Dan Beam 2016/04/07 19:29:54 onload calls chrome.send()[1][2], which does CallJ Show quoted text On 2016/04/07 19:12:21, Dan Beam wrote: > On 2016/04/07 19:11:38, sky wrote: > > On 2016/04/07 01:02:35, Dan Beam wrote: > > > On 2016/04/07 00:59:55, dpapad wrote: > > > > Nit(optional): Perhaps adding a comment to the effect of > > > > "Force some async operations to happen synchronously" would help future > > > readers. > > > > > > well, it's named Polymer.dom.flush(), i.e. flush Polymer-based DOM. also, > > just > > > googling is probably better / more future proof. > > > > Actually, one question. Why isn't ui_test_utils::NavigateToURL enough? > > because the list is rendered after onload onload calls chrome.send()[1][2], which does CallJavascriptFunction()[3][4], IPCs the data via JS[5] and asynchronously queues rendering[6][7][8][9][10][11]. I have no idea how this worked with the new downloads page this long, but the very last step ("asynchronously queues rendering") became slightly more async recently[12] and the house of cards crumbled (this test started failing in that CL). [1] https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res... [2] https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res... [3] https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/... [4] https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/... [5] https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res... [6] https://code.google.com/p/chromium/codesearch#chromium/src/third_party/polyme... [7] https://github.com/Polymer/polymer/blob/b30f962d3950ee2fbb9be5a63ec442885af6c... [8] https://github.com/Polymer/polymer/blob/edb59eb6283074ce54f374b30b884c9f9fcf9... [9] https://github.com/Polymer/polymer/blob/b30f962d3950ee2fbb9be5a63ec442885af6c... [10] https://github.com/Polymer/polymer/blob/b30f962d3950ee2fbb9be5a63ec442885af6c... [11] https://github.com/Polymer/polymer/blob/b30f962d3950ee2fbb9be5a63ec442885af6c... [12] https://codereview.chromium.org/1862213002/diff/40001/third_party/polymer/v1_...
339 EXPECT_EQ(1, FindInPageASCII(web_contents, download_url.spec(),	340 EXPECT_EQ(1, FindInPageASCII(web_contents, download_url.spec(),

340 kFwd, kIgnoreCase, NULL));	341 kFwd, kIgnoreCase, NULL));

341 }	342 }

342	343

343 // Verify search selection coordinates. The data file used is set-up such that	344 // Verify search selection coordinates. The data file used is set-up such that

344 // the text occurs on the same line, and we verify their positions by verifying	345 // the text occurs on the same line, and we verify their positions by verifying

345 // their relative positions.	346 // their relative positions.

346 IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, FindInPageSpecialURLs) {	347 IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, FindInPageSpecialURLs) {

347 const std::wstring search_string(L"\u5728\u897f\u660c\u536b\u661f\u53d1");	348 const std::wstring search_string(L"\u5728\u897f\u660c\u536b\u661f\u53d1");

348 gfx::Rect first, second, first_reverse;	349 gfx::Rect first, second, first_reverse;

(...skipping 1197 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
1546	1547

1547 EXPECT_TRUE(chrome::ExecuteCommand(browser_incognito, IDC_FIND_NEXT));	1548 EXPECT_TRUE(chrome::ExecuteCommand(browser_incognito, IDC_FIND_NEXT));

1548 WebContents* web_contents_incognito =	1549 WebContents* web_contents_incognito =

1549 browser_incognito->tab_strip_model()->GetActiveWebContents();	1550 browser_incognito->tab_strip_model()->GetActiveWebContents();

1550 ui_test_utils::FindInPageNotificationObserver observer(	1551 ui_test_utils::FindInPageNotificationObserver observer(

1551 web_contents_incognito);	1552 web_contents_incognito);

1552 observer.Wait();	1553 observer.Wait();

1553 EXPECT_EQ(ASCIIToUTF16("bar"),	1554 EXPECT_EQ(ASCIIToUTF16("bar"),

1554 GetFindBarTextForBrowser(browser_incognito));	1555 GetFindBarTextForBrowser(browser_incognito));

1555 }	1556 }

OLD	NEW