Add elevation to the managed mode navigation observer.

chrome/browser/managed_mode/managed_mode_navigation_observer.cc

Index: chrome/browser/managed_mode/managed_mode_navigation_observer.cc
diff --git a/chrome/browser/managed_mode/managed_mode_navigation_observer.cc b/chrome/browser/managed_mode/managed_mode_navigation_observer.cc
index af66cf33aa5be0f03abd13c420302de2ee808fe3..aa60fb311385085d20304f72c286bb8e6244f8da 100644
--- a/chrome/browser/managed_mode/managed_mode_navigation_observer.cc
+++ b/chrome/browser/managed_mode/managed_mode_navigation_observer.cc
@@ -263,10 +263,13 @@ ManagedModeNavigationObserver::ManagedModeNavigationObserver(
       preview_infobar_delegate_(NULL),
       got_user_gesture_(false),
       state_(RECORDING_URLS_BEFORE_PREVIEW),
+      is_elevated_(false),
       last_allowed_page_(-1) {
   Profile* profile =
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
   managed_user_service_ = ManagedUserServiceFactory::GetForProfile(profile);
+  if (!managed_user_service_->ProfileIsManaged())
+    is_elevated_ = true;
   url_filter_ = managed_user_service_->GetURLFilterForUIThread();
 }
 
@@ -341,6 +344,14 @@ void ManagedModeNavigationObserver::AddSavedURLsToWhitelistAndClearState() {
   ClearObserverState();
 }
 
+bool ManagedModeNavigationObserver::is_elevated() const {
+  return is_elevated_;
+}
+
+void ManagedModeNavigationObserver::set_elevated(bool is_elevated) {
+  is_elevated_ = is_elevated;
+}
+
 void ManagedModeNavigationObserver::AddURLToPatternList(const GURL& url) {
   navigated_urls_.insert(url);
   last_url_ = url;
@@ -355,6 +366,31 @@ bool ManagedModeNavigationObserver::CanTemporarilyNavigateHost(
   return last_url_.host() == url.host();
 }
 
+namespace {
+const char kChromeWebstoreURL[] = "chrome.google.com/webstore/";

[Bernhard Bauer, 2013/03/21 15:52:58]

1. Move constant declarations to the top of the file.
2. Const variables automatically get internal linkage, so an anonymous namespace
isn't necessary.
3. Isn't this declared as a constant somewhere?

[Adrian Kuegel, 2013/03/21 16:30:04]

1. Done
2. Still some reviewers (for example Jochen) want to have it in an anonymous
namespace. But I can remove it if you want.
3. Yes, it is, and also in many places (usually with a more specific URL). In
one case it is the URL I need, but it is named GalleryBrowsePrefix (it is in the
file extension_constants.h). Should I use that one instead?

[Bernhard Bauer, 2013/03/22 09:24:28]

Meh. My take on this: if there is nothing else that warrants an anonymous
namespace, adding one is just unnecessary noise. If there already is one, it's
fine to add it inside.
I'll have to see how the end result with the host checks look, but using an
already-existing constant would definitely be preferable. 

[Adrian Kuegel, 2013/03/22 09:46:21]

Ok, I am using this constant now.

+}
+
+bool ManagedModeNavigationObserver::WillStayElevatedForURL(
+    const GURL& navigation_url) {
+  // chrome:// URLs have chrome as a host type in the GURL. But all
+  // chrome URLs in url_constants.h are specified with a chrome:// prefix.
+  // Therefore we insert a ":/" between host and path for the chrome:// URLs.
+  std::string url =
+      navigation_url.host() +
+      (navigation_url.host() == "chrome" ? ":/" : "") +
+      navigation_url.path();

[Bernhard Bauer, 2013/03/21 15:52:58]

Sorry, I don't understand this. I'm also thinking there must be a way to do this
that is less string-fiddly. Doesn't url_constants.h also declare just hostnames
for the chrome:// URLs? Could you just check those?

[Adrian Kuegel, 2013/03/21 16:30:04]

True, I can use the host. Thanks for the tip :)

+
+  // Check if any of the special urls is a prefix of url. This is the case if
+  // the special url occurs at position 0 as a substring of url.

[Bernhard Bauer, 2013/03/21 15:52:58]

base/string_util.h declares StartsWith() methods.

[Adrian Kuegel, 2013/03/21 16:30:04]

Ok, I used that instead. It is probably easier to read that way.

+  return url.find(kChromeWebstoreURL) == 0 ||
+      url.find(chrome::kChromeUIHistoryURL) == 0 ||
+      url.find(chrome::kChromeUIHistoryFrameURL) == 0 ||

[Bernhard Bauer, 2013/03/21 15:52:58]

If we only call this method for main frames, why do we need to check for the
subframe URLs?

[Adrian Kuegel, 2013/03/21 16:30:04]

A user could enter the subframe URL manually. But I guess we don't need to
handle that, so I removed that now.

+      url.find(chrome::kChromeUIExtensionsURL) == 0 ||
+      url.find(chrome::kChromeUIExtensionsFrameURL) == 0 ||
+      url.find(chrome::kChromeUISettingsURL) == 0 ||
+      url.find(chrome::kChromeUISettingsFrameURL) == 0;
+}
+
 void ManagedModeNavigationObserver::ClearObserverState() {
   if (preview_infobar_delegate_) {
     InfoBarService* infobar_service =
@@ -387,6 +423,8 @@ void ManagedModeNavigationObserver::NavigateToPendingEntry(
 void ManagedModeNavigationObserver::DidNavigateMainFrame(
     const content::LoadCommittedDetails& details,
     const content::FrameNavigateParams& params) {
+  if (!WillStayElevatedForURL(params.url))
+    is_elevated_ = false;
 
   content::RecordAction(UserMetricsAction("ManagedMode_MainFrameNavigation"));
 
@@ -438,13 +476,14 @@ void ManagedModeNavigationObserver::DidStartProvisionalLoadForFrame(
     bool is_error_page,
     bool is_iframe_srcdoc,
     content::RenderViewHost* render_view_host) {
-  if (!is_main_frame)
-    return;
 }
 
 void ManagedModeNavigationObserver::ProvisionalChangeToMainFrameUrl(
     const GURL& url,
     content::RenderViewHost* render_view_host) {
+  if (!WillStayElevatedForURL(url))

[Bernhard Bauer, 2013/03/21 15:52:58]

Do we actually need both of these checks?

[Adrian Kuegel, 2013/03/21 16:30:04]

Yes. There are two cases: we do a real navigation, then
ProvisionalChangeToMainFrameURL is called before the interstitial is shown (so
we can still make sure to remove the elevation in time). The other case is: we
are on one of these pages with the uber frame, and navigate there (for example
to the help page). Then, only the other method is called, but this does not
bother us because we don't show an interstitial on chrome pages, anyway (still
we want to remove the elevation).

+    is_elevated_ = false;
+
   // This function is the last one to be called before the resource throttle
   // shows the interstitial if the URL must be blocked.
   DVLOG(1) << "ProvisionalChangeToMainFrameURL " << url.spec();