Add reverse URL handler for shortening uber URLs

content/browser/web_contents/navigation_controller_impl.cc

 // Copyright (c) 2012 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 "content/browser/web_contents/navigation_controller_impl.h"
 
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/string_number_conversions.h"  // Temporary
 #include "base/string_util.h"
@@ skipping 905 matching lines @@
     // case! We should have some more tracking to know for sure.
     new_entry = new NavigationEntryImpl(*pending_entry_);
 
     // Don't use the page type from the pending entry. Some interstitial page
     // may have set the type to interstitial. Once we commit, however, the page
     // type must always be normal.
     new_entry->set_page_type(content::PAGE_TYPE_NORMAL);
     update_virtual_url = new_entry->update_virtual_url_with_url();
   } else {
     new_entry = new NavigationEntryImpl;
+
+    // Find out whether the new entry needs to update its virtual URL on URL
+    // change and set up the entry accordingly. This is needed to correctly
+    // update the virtual URL when replaceState is called after a pushState.

[Charlie Reis, 2012/09/13 22:10:09]

We were always doing this before, by using true on line 930.  Why do we need to
narrow it to only the cases that RewriteURLIfNecessary sets reverse_on_redirect
to true?  (I find this harder to follow and don't understand the extra
complexity.)

[Cem Kocagil, 2012/09/13 22:40:03]

Actually the only thing that is of importance to me here is the
"set_update_virtual_url_with_url" call. Since I find whether the virtual URL
needs to be updated, I thought I could set "update_virtual_url" to that value
too (instead of always true).

I can revert that line if you want, it doesn't affect the rest of this patch.

[Charlie Reis, 2012/09/13 22:58:23]

Oh, I didn't realize that was the important part.  Feel free to leave the change
to the update_virtual_url line.

Perhaps this would be clearer if you renamed the bool to needs_update? 
(reverse_on_redirect isn't self-evident here.)

+    GURL temp = params.url;

[Charlie Reis, 2012/09/13 22:10:09]

nit: Please use a better variable name than temp.

+    bool reverse_on_redirect = false;
+    BrowserURLHandlerImpl::GetInstance()->RewriteURLIfNecessary(
+        &temp, browser_context_, &reverse_on_redirect);
+    new_entry->set_update_virtual_url_with_url(reverse_on_redirect);
+
     // When navigating to a new page, give the browser URL handler a chance to
     // update the virtual URL based on the new URL. For example, this is needed
     // to show chrome://bookmarks/#1 when the bookmarks webui extension changes
     // the URL.
-    update_virtual_url = true;
+    update_virtual_url = reverse_on_redirect;
   }
 
   new_entry->SetURL(params.url);
   if (update_virtual_url)
     UpdateVirtualURLToURL(new_entry, params.url);
   new_entry->SetReferrer(params.referrer);
   new_entry->SetPageID(params.page_id);
   new_entry->SetTransitionType(params.transition);
   new_entry->set_site_instance(
       static_cast<SiteInstanceImpl*>(web_contents_->GetSiteInstance()));
@@ skipping 623 matching lines @@
   for (int i = 0; i < max_index; i++) {
     // When cloning a tab, copy all entries except interstitial pages
     if (source.entries_[i].get()->GetPageType() !=
         content::PAGE_TYPE_INTERSTITIAL) {
       entries_.insert(entries_.begin() + insert_index++,
                       linked_ptr<NavigationEntryImpl>(
                           new NavigationEntryImpl(*source.entries_[i])));
     }
   }
 }