diff options
author | jered@chromium.org <jered@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-10-01 01:33:30 +0000 |
---|---|---|
committer | jered@chromium.org <jered@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-10-01 01:33:30 +0000 |
commit | 2309e91187adda1de77bddebc6e192f9d6c3ed43 (patch) | |
tree | 57808253b2529cf7455162956f52d731a91ba2f0 /chrome/common | |
parent | bc076021f25f48d6de00cd9cd6bf9575d8e21118 (diff) | |
download | chromium_src-2309e91187adda1de77bddebc6e192f9d6c3ed43.zip chromium_src-2309e91187adda1de77bddebc6e192f9d6c3ed43.tar.gz chromium_src-2309e91187adda1de77bddebc6e192f9d6c3ed43.tar.bz2 |
InstantExtended: Send search URLs to renderers.
Previously, navigations initiated by an instant renderer were bounced to
the browser to be rebucketed into an instant or non-instant renderer.
But navigations from non-instant renderers to instant URLs (i.e.
privileged search page URLs) were not rebucketed, because the renderer
had no knowledge of which URLs were instant URLs, and it would be too
expensive to bounce ALL URLs. As a result, non-instant pages like
google.com/setprefs could not redirect to instant pages like
google.com/search.
This change has InstantService tell renderers about URLs associated with
the default search engine, and uses this knowledge to bounce
renderer-initiated navigations to likely instant URLs from non-instant
processes back into the browser for rebucketing. So now link clicks from
non-instant pages to instant pages will put the destination pages into
an instant process. Unfortunately, though, redirects are still broken.
For example,
0. User clicks "Set preferences" button on an instant page.
1. Instant renderer bounces google.com/setprefs to the browser.
2. Browser says google.com/setprefs is not an instant URL,
and assigns it to a non-instant renderer.
3. After rebucketing to the non-instant renderer, google.com/setprefs
is marked as a browser initiated request(!)
4. /setprefs redirects to /search, which _is_ an instant URL, but
because /setprefs is marked as browser initiated the non-instant
renderer does not get a chance to bounce it back to the browser.
I tried working around this by giving redirects a chance to bounce back
to the browser in step #4, but this broke the signin process model in a
scary way that I don't fully understand. It seems like the right fix
here is going to need to EITHER follow the redirect chain in step #2
when determining if an URL is an instant URL, OR somehow flag the
bounced redirect request for further inspection in step #3.
This change also includes a small side benefit. Since renderers now know
about instant URLs, we can suppress a flash of a baked-in error page in
the event of an error while loading the InstantExtended new tab page.
TEST=manual,unit,browsertest
BUG=271088,223754
Review URL: https://codereview.chromium.org/23455047
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@226103 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/common')
-rw-r--r-- | chrome/common/render_messages.h | 6 | ||||
-rw-r--r-- | chrome/common/search_urls.cc | 26 | ||||
-rw-r--r-- | chrome/common/search_urls.h | 17 | ||||
-rw-r--r-- | chrome/common/search_urls_unittest.cc | 40 |
4 files changed, 89 insertions, 0 deletions
diff --git a/chrome/common/render_messages.h b/chrome/common/render_messages.h index a9a2a98..5fadf01 100644 --- a/chrome/common/render_messages.h +++ b/chrome/common/render_messages.h @@ -756,6 +756,12 @@ IPC_MESSAGE_ROUTED2(ChromeViewHostMsg_SetVoiceSearchSupported, int /* page_id */, bool /* supported */) +// Tells the renderer a list of URLs which should be bounced back to the browser +// process so that they can be assigned to an Instant renderer. +IPC_MESSAGE_CONTROL2(ChromeViewMsg_SetSearchURLs, + std::vector<GURL> /* search_urls */, + GURL /* new_tab_page_url */) + // Tells listeners that a detailed message was reported to the console by // WebKit. IPC_MESSAGE_ROUTED4(ChromeViewHostMsg_DetailedConsoleMessageAdded, diff --git a/chrome/common/search_urls.cc b/chrome/common/search_urls.cc new file mode 100644 index 0000000..9856842 --- /dev/null +++ b/chrome/common/search_urls.cc @@ -0,0 +1,26 @@ +// Copyright 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/common/search_urls.h" + +#include "content/public/common/url_constants.h" +#include "url/gurl.h" + +namespace search { + +namespace { +bool MatchesOrigin(const GURL& my_url, const GURL& other_url) { + return my_url.host() == other_url.host() && + my_url.port() == other_url.port() && + (my_url.scheme() == other_url.scheme() || + (my_url.SchemeIs(content::kHttpsScheme) && + other_url.SchemeIs(content::kHttpScheme))); +} +} // namespace + +bool MatchesOriginAndPath(const GURL& my_url, const GURL& other_url) { + return MatchesOrigin(my_url, other_url) && my_url.path() == other_url.path(); +} + +} // namespace search diff --git a/chrome/common/search_urls.h b/chrome/common/search_urls.h new file mode 100644 index 0000000..c4bbacd --- /dev/null +++ b/chrome/common/search_urls.h @@ -0,0 +1,17 @@ +// Copyright 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_COMMON_SEARCH_URLS_H_ +#define CHROME_COMMON_SEARCH_URLS_H_ + +class GURL; + +namespace search { + +// Returns true if |my_url| matches |other_url|. +bool MatchesOriginAndPath(const GURL& my_url, const GURL& other_url); + +} // namespace search + +#endif // CHROME_COMMON_SEARCH_URLS_H_ diff --git a/chrome/common/search_urls_unittest.cc b/chrome/common/search_urls_unittest.cc new file mode 100644 index 0000000..ceb98ef --- /dev/null +++ b/chrome/common/search_urls_unittest.cc @@ -0,0 +1,40 @@ +// Copyright 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/common/search_urls.h" + +#include "testing/gtest/include/gtest/gtest.h" +#include "url/gurl.h" + +namespace search { +typedef testing::Test SearchURLsTest; + +TEST_F(SearchURLsTest, MatchesOriginAndPath) { + EXPECT_TRUE(MatchesOriginAndPath( + GURL("http://example.com/path"), + GURL("http://example.com/path?param"))); + EXPECT_FALSE(MatchesOriginAndPath( + GURL("http://not.example.com/path"), + GURL("http://example.com/path"))); + EXPECT_TRUE(MatchesOriginAndPath( + GURL("http://example.com:80/path"), + GURL("http://example.com/path"))); + EXPECT_FALSE(MatchesOriginAndPath( + GURL("http://example.com:8080/path"), + GURL("http://example.com/path"))); + EXPECT_FALSE(MatchesOriginAndPath( + GURL("ftp://example.com/path"), + GURL("http://example.com/path"))); + EXPECT_FALSE(MatchesOriginAndPath( + GURL("http://example.com/path"), + GURL("https://example.com/path"))); + EXPECT_TRUE(MatchesOriginAndPath( + GURL("https://example.com/path"), + GURL("http://example.com/path"))); + EXPECT_FALSE(MatchesOriginAndPath( + GURL("http://example.com/path"), + GURL("http://example.com/another-path"))); +} + +} // namespace search |