Skip to content

Commit

Permalink
Revert of Classify navigations without page id in parallel to the exi…
Browse files Browse the repository at this point in the history
…sting classifier. (patchset chromium#37 id:720001 of https://codereview.chromium.org/1002803002/)

Reason for revert:
This patch broke missing-repaint-after-slow-style-sheet.pl, see crbug.com/484531 for more information.

Original issue's description:
> Classify navigations without page id in parallel to the existing classifier.
>
> For now, this only happens in debug builds.
>
> BUG=369661
> TEST=NavigationControllerBrowserTest.NavigationTypeClassification_*
> TEST=Every other test on the planet.
>
> Committed: https://crrev.com/d8d93348bbd8c646c337bdaa40fc0c64204fc5ff
> Cr-Commit-Position: refs/heads/master@{#327122}
>
> Reverted: https://crrev.com/5348e920f4119aff9a4eb76c0965725dc85a66cc
> Cr-Revert-Position: refs/heads/master@{#327152}
>
> Committed: https://crrev.com/5671403d44971669e4d81aecf3f002188ce0e95f
> Cr-Commit-Position: refs/heads/master@{#327214}
>
> Reverted: https://crrev.com/49180eb13549e440bbd4f66390e32e84699dcdfd
> Cr-Revert-Position: refs/heads/master@{#327269}
>
> Committed: https://crrev.com/a038c6670f450313a8e224ccc5d05dd59e3488c4
> Cr-Commit-Position: refs/heads/master@{#328131}

TBR=phajdan.jr@chromium.org,clamy@chromium.org,creis@chromium.org,hajimehoshi@chromium.org,isherman@chromium.org,jeremyim@chromium.org,mattm@chromium.org,mnaganov@chromium.org,mvanouwerkerk@chromium.org,nasko@chromium.org,stevenjb@chromium.org,bengr@chromium.org,cpu@chromium.org,nkostylev@chromium.org,avi@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=369661,484531

Review URL: https://codereview.chromium.org/1121083004

Cr-Commit-Position: refs/heads/master@{#328282}
  • Loading branch information
progers authored and Commit bot committed May 5, 2015
1 parent 47870b5 commit 1f5d619
Show file tree
Hide file tree
Showing 41 changed files with 512 additions and 1,277 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "content/public/browser/interstitial_page.h"
#include "content/public/browser/navigation_controller.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/web_contents.h"
#include "content/public/test/web_contents_tester.h"

Expand Down Expand Up @@ -65,14 +64,10 @@ class MergeSessionLoadPageTest : public ChromeRenderViewHostTestHarness {
ChromeRenderViewHostTestHarness::TearDown();
}

void Navigate(const char* url,
int page_id,
int nav_entry_id,
bool did_create_new_entry) {
WebContentsTester::For(web_contents())
->TestDidNavigate(web_contents()->GetMainFrame(), page_id, nav_entry_id,
did_create_new_entry, GURL(url),
ui::PAGE_TRANSITION_TYPED);
void Navigate(const char* url, int page_id) {
WebContentsTester::For(web_contents())->TestDidNavigate(
web_contents()->GetMainFrame(), page_id, GURL(url),
ui::PAGE_TRANSITION_TYPED);
}

void ShowInterstitial(const char* url) {
Expand Down Expand Up @@ -122,7 +117,7 @@ class MergeSessionLoadPageTest : public ChromeRenderViewHostTestHarness {
TEST_F(MergeSessionLoadPageTest, MergeSessionPageNotShown) {
SetMergeSessionState(OAuth2LoginManager::SESSION_RESTORE_DONE);
// Start a load.
Navigate(kURL1, 1, 0, true);
Navigate(kURL1, 1);
// Load next page.
controller().LoadURL(GURL(kURL2), content::Referrer(),
ui::PAGE_TRANSITION_TYPED, std::string());
Expand All @@ -140,7 +135,7 @@ TEST_F(MergeSessionLoadPageTest, MergeSessionPageNotShownOnTimeout) {
base::TimeDelta::FromSeconds(kSessionMergeTimeout + 1));

// Start a load.
Navigate(kURL1, 1, 0, true);
Navigate(kURL1, 1);
// Load next page.
controller().LoadURL(GURL(kURL2), content::Referrer(),
ui::PAGE_TRANSITION_TYPED, std::string());
Expand All @@ -155,11 +150,10 @@ TEST_F(MergeSessionLoadPageTest, MergeSessionPageShown) {
SetMergeSessionState(OAuth2LoginManager::SESSION_RESTORE_IN_PROGRESS);

// Start a load.
Navigate(kURL1, 1, 0, true);
Navigate(kURL1, 1);
// Load next page.
controller().LoadURL(GURL(kURL2), content::Referrer(),
ui::PAGE_TRANSITION_TYPED, std::string());
int pending_id = controller().GetPendingEntry()->GetUniqueID();

// Simulate the load causing an merge session interstitial page
// to be shown.
Expand All @@ -176,7 +170,7 @@ TEST_F(MergeSessionLoadPageTest, MergeSessionPageShown) {
EXPECT_EQ(kURL2, web_contents()->GetVisibleURL().spec());

// Commit navigation and the interstitial page is gone.
Navigate(kURL2, 2, pending_id, true);
Navigate(kURL2, 2);
EXPECT_FALSE(GetMergeSessionLoadPage());
}

Expand Down
18 changes: 7 additions & 11 deletions chrome/browser/chromeos/offline/offline_load_page_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,10 @@ class OfflineLoadPageTest : public ChromeRenderViewHostTestHarness {
user_response_ = CANCEL;
}

void Navigate(const char* url,
int page_id,
int nav_entry_id,
bool did_create_new_entry) {
WebContentsTester::For(web_contents())
->TestDidNavigate(web_contents()->GetMainFrame(), page_id, nav_entry_id,
did_create_new_entry, GURL(url),
ui::PAGE_TRANSITION_TYPED);
void Navigate(const char* url, int page_id) {
WebContentsTester::For(web_contents())->TestDidNavigate(
web_contents()->GetMainFrame(), page_id, GURL(url),
ui::PAGE_TRANSITION_TYPED);
}

void ShowInterstitial(const char* url) {
Expand All @@ -95,7 +91,7 @@ void TestOfflineLoadPage::NotifyBlockingPageComplete(bool proceed) {

TEST_F(OfflineLoadPageTest, OfflinePageProceed) {
// Start a load.
Navigate(kURL1, 1, 0, true);
Navigate(kURL1, 1);
// Load next page.
controller().LoadURL(GURL(kURL2), content::Referrer(),
ui::PAGE_TRANSITION_TYPED, std::string());
Expand All @@ -117,14 +113,14 @@ TEST_F(OfflineLoadPageTest, OfflinePageProceed) {
EXPECT_EQ(kURL2, web_contents()->GetVisibleURL().spec());

// Commit navigation and the interstitial page is gone.
Navigate(kURL2, 2, 0, true);
Navigate(kURL2, 2);
EXPECT_FALSE(GetOfflineLoadPage());
}

// Tests showing an offline page and not proceeding.
TEST_F(OfflineLoadPageTest, OfflinePageDontProceed) {
// Start a load.
Navigate(kURL1, 1, 0, true);
Navigate(kURL1, 1);
controller().LoadURL(GURL(kURL2), content::Referrer(),
ui::PAGE_TRANSITION_TYPED, std::string());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
#include "components/infobars/core/infobar.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/navigation_details.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
#include "content/public/browser/notification_service.h"
Expand Down Expand Up @@ -225,9 +224,8 @@ void GeolocationPermissionContextTests::AddNewTab(const GURL& url) {
content::WebContents* new_tab = CreateTestWebContents();
new_tab->GetController().LoadURL(
url, content::Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
content::NavigationEntry* entry = new_tab->GetController().GetPendingEntry();
content::RenderFrameHostTester::For(new_tab->GetMainFrame())
->SendNavigate(extra_tabs_.size() + 1, entry->GetUniqueID(), true, url);
->SendNavigate(extra_tabs_.size() + 1, url);

// Set up required helpers, and make this be as "tabby" as the code requires.
#if defined(ENABLE_EXTENSIONS)
Expand Down
6 changes: 2 additions & 4 deletions chrome/browser/rlz/rlz_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -270,11 +270,9 @@ void RlzLibTest::SimulateHomepageUsage() {
content::RenderFrameHostTester::For(main_rfh());

// Simulate a navigation to homepage first.
rfht->SendNavigateWithTransition(
0, 0, true, home_url, ui::PAGE_TRANSITION_HOME_PAGE);
rfht->SendNavigateWithTransition(0, home_url, ui::PAGE_TRANSITION_HOME_PAGE);
// Then simulate a search from homepage.
rfht->SendNavigateWithTransition(
1, 0, true, search_url, ui::PAGE_TRANSITION_LINK);
rfht->SendNavigateWithTransition(1, search_url, ui::PAGE_TRANSITION_LINK);
}

void RlzLibTest::SimulateAppListUsage() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
#include "components/history/core/browser/history_backend.h"
#include "components/history/core/browser/history_service.h"
#include "content/public/browser/navigation_controller.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/referrer.h"
#include "content/public/test/test_browser_thread.h"
Expand Down Expand Up @@ -133,8 +132,6 @@ class BrowserFeatureExtractorTest : public ChromeRenderViewHostTestHarness {
web_contents()->GetController().LoadURL(
url, content::Referrer(referrer, blink::WebReferrerPolicyDefault),
type, std::string());
int pending_id =
web_contents()->GetController().GetPendingEntry()->GetUniqueID();

static int page_id = 0;
content::RenderFrameHost* rfh =
Expand All @@ -144,7 +141,7 @@ class BrowserFeatureExtractorTest : public ChromeRenderViewHostTestHarness {
}
WebContentsTester::For(web_contents())->ProceedWithCrossSiteNavigation();
WebContentsTester::For(web_contents())->TestDidNavigateWithReferrer(
rfh, ++page_id, pending_id, true, url,
rfh, ++page_id, url,
content::Referrer(referrer, blink::WebReferrerPolicyDefault), type);
}

Expand Down
Loading

0 comments on commit 1f5d619

Please sign in to comment.