Skip to content

Commit

Permalink
Suppress NEW_NAVIGATION_ENTRY prerenderer failures for ORIGIN_OFFLINE
Browse files Browse the repository at this point in the history
We have noticed some FINAL_STATUS_NEW_NAVIGATION_ENTRY failures in UMA
for offline pages but didn't know how to trigger them. Bug 675767 has
identified a site (foxsports.com) with articles/links that do have this
issue. The issue is repeatable for those pages and they are never
succeeding. I end-to-end tested this change with logging to confirm
we do repeatably see entry count going to 2 and that with this change
the pages can background load successfully.

BUG=675767

Review-Url: https://codereview.chromium.org/2656653002
Cr-Commit-Position: refs/heads/master@{#446379}
  • Loading branch information
dougarnett authored and Commit bot committed Jan 26, 2017
1 parent ce00d1f commit 3ae6f73
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 0 deletions.
25 changes: 25 additions & 0 deletions chrome/browser/prerender/prerender_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3110,6 +3110,31 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderNewNavigationEntry) {
FINAL_STATUS_NEW_NAVIGATION_ENTRY, 1);
}

// Checks that the prerendering of a page for ORIGIN_OFFLINE is not canceled
// when the prerendered page tries to make a second navigation entry.
IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
PrerenderNewNavigationEntryForOffline) {
// Navigate to about:blank to get the session storage namespace.
ui_test_utils::NavigateToURL(current_browser(), GURL(url::kAboutBlankURL));
content::SessionStorageNamespace* storage_namespace =
GetActiveWebContents()
->GetController()
.GetDefaultSessionStorageNamespace();

std::unique_ptr<TestPrerender> test_prerender =
prerender_contents_factory()->ExpectPrerenderContents(
FINAL_STATUS_APP_TERMINATING);

const GURL url =
src_server()->GetURL(MakeAbsolute("/prerender/prerender_new_entry.html"));
std::unique_ptr<PrerenderHandle> prerender_handle(
GetPrerenderManager()->AddPrerenderForOffline(url, storage_namespace,
gfx::Size(640, 480)));
ASSERT_EQ(prerender_handle->contents(), test_prerender->contents());
test_prerender->WaitForLoads(2);
ASSERT_EQ(1, GetActiveWebContents()->GetController().GetEntryCount());
}

// Attempt a swap-in in a new tab. The session storage doesn't match, so it
// should not swap.
IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPageNewTab) {
Expand Down
7 changes: 7 additions & 0 deletions chrome/browser/prerender/prerender_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,13 @@ void PrerenderContents::DidFinishLoad(
void PrerenderContents::DidNavigateMainFrame(
const content::LoadCommittedDetails& details,
const content::FrameNavigateParams& params) {
// Prevent ORIGIN_OFFLINE prerenders from being destroyed on location.href
// change, since the history is never merged for offline prerenders. Also
// avoid adding aliases as they may potentially mark other valid requests to
// offline as duplicate.
if (origin() == ORIGIN_OFFLINE)
return;

// If the prerender made a second navigation entry, abort the prerender. This
// avoids having to correctly implement a complex history merging case (this
// interacts with location.replace) and correctly synchronize with the
Expand Down

0 comments on commit 3ae6f73

Please sign in to comment.