PlzNavigate: make SitePerProcessBrowserTest.NavigateRemoteAfterError work

PlzNavigate: make SitePerProcessBrowserTest.NavigateRemoteAfterError work

This CL fixes an issue with SitePerProcessBrowserTest.NavigateRemoteAfterError
that was preventing it from passing with PlzNavigate enabled. The test was
mixing TestNavigationObservers with TestFrameNavigationObservers, which
resulted in calling NavigateIframeToURL while the frame was still considered as
loading when PlzNavigate is enabled. Hence WebContents was never notifying its
observer that it started loading due to the new navigation. When the navigation
would stop, TestNavigationObserver would discard the stop since it considered
thta the navigation never started. Now the test waits until the navigation has
stopped in the whole tree before calling NavigateIframeToURL.

BUG=475027
Committed: https://crrev.com/7531fd26bbaa1ed8a4041d8fb15983463eeab70e
Cr-Commit-Position: refs/heads/master@{#377873}

[clamy, 2016-02-22 16:41:21]

Description was changed from

==========
PlzNavigate: make SitePerProcessBrowserTest.NavigateRemoteAfterError work

This CL fixes an issue with SitePerProcessBrowserTest.NavigateRemoteAfterError
that was preventing it from passing with PlzNavigate enabled. The test was
mixing TestNavigationObservers with TestFrameNavigationObservers, which
resulted in calling NavigateIframeToURL while the frame was still considered as
loading when PlzNavigate is enabled. Hence WebContents was never notifying its
observer that it started loading due to the new navigation. When the navigation
would stop, TestNavigationObserver would discard the stop since it considered
thta the navigation never started. Now the test waits until the navigation has
stopped in the whole tree before calling NavigateIframeToURL.

BUG=475027
==========

to

==========
PlzNavigate: make SitePerProcessBrowserTest.NavigateRemoteAfterError work

This CL fixes an issue with SitePerProcessBrowserTest.NavigateRemoteAfterError
that was preventing it from passing with PlzNavigate enabled. The test was
mixing TestNavigationObservers with TestFrameNavigationObservers, which
resulted in calling NavigateIframeToURL while the frame was still considered as
loading when PlzNavigate is enabled. Hence WebContents was never notifying its
observer that it started loading due to the new navigation. When the navigation
would stop, TestNavigationObserver would discard the stop since it considered
thta the navigation never started. Now the test waits until the navigation has
stopped in the whole tree before calling NavigateIframeToURL.

BUG=475027
==========

[clamy, 2016-02-22 16:41:21]

clamy@chromium.org changed reviewers:
+ nasko@chromium.org

[clamy, 2016-02-22 16:41:32]

@nasko: PTAL.

[nasko, 2016-02-25 19:58:41]

LGTM with a nit.

https://codereview.chromium.org/1722613002/diff/20001/content/browser/site_pe...
File content/browser/site_per_process_browsertest.cc (right):

https://codereview.chromium.org/1722613002/diff/20001/content/browser/site_pe...
content/browser/site_per_process_browsertest.cc:1345: 
nit: No need for two empty lines. Remove one of them.

https://codereview.chromium.org/1722613002/diff/20001/content/browser/site_pe...
content/browser/site_per_process_browsertest.cc:1380: GURL url =
embedded_test_server()->GetURL("bar.com", "/title3.html");
nit: Since all navigations are done now in local scopes, let's put this one also
in a local scope for consistency.

[clamy, 2016-02-26 11:04:17]

Thanks!

https://codereview.chromium.org/1722613002/diff/20001/content/browser/site_pe...
File content/browser/site_per_process_browsertest.cc (right):

https://codereview.chromium.org/1722613002/diff/20001/content/browser/site_pe...
content/browser/site_per_process_browsertest.cc:1345: 
On 2016/02/25 19:58:40, nasko wrote:
> nit: No need for two empty lines. Remove one of them.

Done.

https://codereview.chromium.org/1722613002/diff/20001/content/browser/site_pe...
content/browser/site_per_process_browsertest.cc:1380: GURL url =
embedded_test_server()->GetURL("bar.com", "/title3.html");
On 2016/02/25 19:58:41, nasko wrote:
> nit: Since all navigations are done now in local scopes, let's put this one
also
> in a local scope for consistency.

Done.

[clamy, 2016-02-26 11:04:21]

The CQ bit was checked by clamy@chromium.org

[clamy, 2016-02-26 11:04:21]

The patchset sent to the CQ was uploaded after l-g-t-m from nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/1722613002/#ps40001
(title: "Rebase + addressed comments")

[commit-bot: I haz the power, 2016-02-26 11:04:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1722613002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1722613002/40001

[commit-bot: I haz the power, 2016-02-26 12:37:22]

Description was changed from

==========
PlzNavigate: make SitePerProcessBrowserTest.NavigateRemoteAfterError work

This CL fixes an issue with SitePerProcessBrowserTest.NavigateRemoteAfterError
that was preventing it from passing with PlzNavigate enabled. The test was
mixing TestNavigationObservers with TestFrameNavigationObservers, which
resulted in calling NavigateIframeToURL while the frame was still considered as
loading when PlzNavigate is enabled. Hence WebContents was never notifying its
observer that it started loading due to the new navigation. When the navigation
would stop, TestNavigationObserver would discard the stop since it considered
thta the navigation never started. Now the test waits until the navigation has
stopped in the whole tree before calling NavigateIframeToURL.

BUG=475027
==========

to

==========
PlzNavigate: make SitePerProcessBrowserTest.NavigateRemoteAfterError work

This CL fixes an issue with SitePerProcessBrowserTest.NavigateRemoteAfterError
that was preventing it from passing with PlzNavigate enabled. The test was
mixing TestNavigationObservers with TestFrameNavigationObservers, which
resulted in calling NavigateIframeToURL while the frame was still considered as
loading when PlzNavigate is enabled. Hence WebContents was never notifying its
observer that it started loading due to the new navigation. When the navigation
would stop, TestNavigationObserver would discard the stop since it considered
thta the navigation never started. Now the test waits until the navigation has
stopped in the whole tree before calling NavigateIframeToURL.

BUG=475027
==========

[commit-bot: I haz the power, 2016-02-26 12:37:22]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-26 12:38:42]

Description was changed from

==========
PlzNavigate: make SitePerProcessBrowserTest.NavigateRemoteAfterError work

This CL fixes an issue with SitePerProcessBrowserTest.NavigateRemoteAfterError
that was preventing it from passing with PlzNavigate enabled. The test was
mixing TestNavigationObservers with TestFrameNavigationObservers, which
resulted in calling NavigateIframeToURL while the frame was still considered as
loading when PlzNavigate is enabled. Hence WebContents was never notifying its
observer that it started loading due to the new navigation. When the navigation
would stop, TestNavigationObserver would discard the stop since it considered
thta the navigation never started. Now the test waits until the navigation has
stopped in the whole tree before calling NavigateIframeToURL.

BUG=475027
==========

to

==========
PlzNavigate: make SitePerProcessBrowserTest.NavigateRemoteAfterError work

This CL fixes an issue with SitePerProcessBrowserTest.NavigateRemoteAfterError
that was preventing it from passing with PlzNavigate enabled. The test was
mixing TestNavigationObservers with TestFrameNavigationObservers, which
resulted in calling NavigateIframeToURL while the frame was still considered as
loading when PlzNavigate is enabled. Hence WebContents was never notifying its
observer that it started loading due to the new navigation. When the navigation
would stop, TestNavigationObserver would discard the stop since it considered
thta the navigation never started. Now the test waits until the navigation has
stopped in the whole tree before calling NavigateIframeToURL.

BUG=475027

Committed: https://crrev.com/7531fd26bbaa1ed8a4041d8fb15983463eeab70e
Cr-Commit-Position: refs/heads/master@{#377873}
==========

[commit-bot: I haz the power, 2016-02-26 12:38:43]

Patchset 3 (id:??) landed as
https://crrev.com/7531fd26bbaa1ed8a4041d8fb15983463eeab70e
Cr-Commit-Position: refs/heads/master@{#377873}