Skip to content

Commit

Permalink
View source after POST command isn't what you expected.
Browse files Browse the repository at this point in the history 
BUG=523

Review URL: http://codereview.chromium.org/5716003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@69246 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
pfeldman@chromium.org committed Dec 15, 2010
1 parent 792cdcf commit 77d8d62
Show file tree
Hide file tree
Showing 9 changed files with 201 additions and 65 deletions.
17 changes: 9 additions & 8 deletions chrome/browser/tab_contents/navigation_controller.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -986,30 +986,33 @@ void NavigationController::CopyStateFromAndPrune(NavigationController* source) {
}

void NavigationController::PruneAllButActive() {
int prune_count = entry_count();
if (transient_entry_index_ != -1) {
// There is a transient entry. Prune up to it.
DCHECK_EQ(entry_count() - 1, transient_entry_index_);
prune_count = transient_entry_index_;
entries_.erase(entries_.begin(), entries_.begin() + transient_entry_index_);
transient_entry_index_ = 0;
last_committed_entry_index_ = -1;
pending_entry_index_ = -1;
} else if (!pending_entry_) {
// There's no pending entry. Leave the last entry (if there is one).
if (!prune_count)
if (!entry_count())
return;

prune_count--;
DCHECK(last_committed_entry_index_ >= 0);
entries_.erase(entries_.begin(),
entries_.begin() + last_committed_entry_index_);
entries_.erase(entries_.begin() + 1, entries_.end());
last_committed_entry_index_ = 0;
} else if (pending_entry_index_ != -1) {
DCHECK_EQ(pending_entry_index_, prune_count - 1);
entries_.erase(entries_.begin(), entries_.begin() + pending_entry_index_);
entries_.erase(entries_.begin() + 1, entries_.end());
pending_entry_index_ = 0;
last_committed_entry_index_ = 0;
prune_count--;
} else {
// There is a pending_entry, but it's not in entries_.
pending_entry_index_ = -1;
last_committed_entry_index_ = -1;
entries_.clear();
}

if (tab_contents_->interstitial_page()) {
Expand All @@ -1018,8 +1021,6 @@ void NavigationController::PruneAllButActive() {
// so the interstitial triggers a reload if the user doesn't proceed.
tab_contents_->interstitial_page()->set_reload_on_dont_proceed(true);
}

entries_.erase(entries_.begin(), entries_.begin() + prune_count);
}

void NavigationController::DiscardNonCommittedEntries() {
Expand Down
86 changes: 86 additions & 0 deletions chrome/browser/tab_contents/navigation_controller_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1865,6 +1865,92 @@ TEST_F(NavigationControllerTest, HistoryNavigate) {
EXPECT_TRUE(message == NULL);
}

// Test call to PruneAllButActive for the only entry.
TEST_F(NavigationControllerTest, PruneAllButActiveForSingle) {
const GURL url1("http://foo1");
NavigateAndCommit(url1);
controller().PruneAllButActive();

EXPECT_EQ(-1, controller().pending_entry_index());
EXPECT_EQ(controller().GetEntryAtIndex(0)->url(), url1);
}

// Test call to PruneAllButActive for last entry.
TEST_F(NavigationControllerTest, PruneAllButActiveForLast) {
const GURL url1("http://foo1");
const GURL url2("http://foo2");
const GURL url3("http://foo3");

NavigateAndCommit(url1);
NavigateAndCommit(url2);
NavigateAndCommit(url3);
controller().GoBack();
controller().GoBack();
contents()->CommitPendingNavigation();

controller().PruneAllButActive();

EXPECT_EQ(-1, controller().pending_entry_index());
EXPECT_EQ(controller().GetEntryAtIndex(0)->url(), url1);
}

// Test call to PruneAllButActive for intermediate entry.
TEST_F(NavigationControllerTest, PruneAllButActiveForIntermediate) {
const GURL url1("http://foo1");
const GURL url2("http://foo2");
const GURL url3("http://foo3");

NavigateAndCommit(url1);
NavigateAndCommit(url2);
NavigateAndCommit(url3);
controller().GoBack();
contents()->CommitPendingNavigation();

controller().PruneAllButActive();

EXPECT_EQ(-1, controller().pending_entry_index());
EXPECT_EQ(controller().GetEntryAtIndex(0)->url(), url2);
}

// Test call to PruneAllButActive for intermediate entry.
TEST_F(NavigationControllerTest, PruneAllButActiveForPending) {
const GURL url1("http://foo1");
const GURL url2("http://foo2");
const GURL url3("http://foo3");

NavigateAndCommit(url1);
NavigateAndCommit(url2);
NavigateAndCommit(url3);
controller().GoBack();

controller().PruneAllButActive();

EXPECT_EQ(0, controller().pending_entry_index());
}

// Test call to PruneAllButActive for transient entry.
TEST_F(NavigationControllerTest, PruneAllButActiveForTransient) {
const GURL url0("http://foo0");
const GURL url1("http://foo1");
const GURL transient_url("http://transient");

controller().LoadURL(url0, GURL(), PageTransition::TYPED);
rvh()->SendNavigate(0, url0);
controller().LoadURL(url1, GURL(), PageTransition::TYPED);
rvh()->SendNavigate(1, url1);

// Adding a transient with no pending entry.
NavigationEntry* transient_entry = new NavigationEntry;
transient_entry->set_url(transient_url);
controller().AddTransientEntry(transient_entry);

controller().PruneAllButActive();

EXPECT_EQ(-1, controller().pending_entry_index());
EXPECT_EQ(-1, controller().pending_entry_index());
EXPECT_EQ(controller().GetTransientEntry()->url(), transient_url);
}

/* TODO(brettw) These test pass on my local machine but fail on the XP buildbot
(but not Vista) cleaning up the directory after they run.
This should be fixed.
Expand Down
3 changes: 1 addition & 2 deletions chrome/browser/tab_contents/render_view_context_menu.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1216,8 +1216,7 @@ void RenderViewContextMenu::ExecuteCommand(int id) {
break;

case IDC_VIEW_SOURCE:
OpenURL(GURL(chrome::kViewSourceScheme + std::string(":") +
params_.page_url.spec()), NEW_FOREGROUND_TAB, PageTransition::LINK);
source_tab_contents_->ViewSource();
break;

case IDC_CONTENT_CONTEXT_INSPECTELEMENT:
Expand Down
6 changes: 6 additions & 0 deletions chrome/browser/tab_contents/tab_contents.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1521,6 +1521,12 @@ int TabContents::GetZoomPercent(bool* enable_increment,
return percent;
}

void TabContents::ViewSource()
{
if (delegate_)
delegate_->ViewSourceForTab(this);
}

// Notifies the RenderWidgetHost instance about the fact that the page is
// loading, or done loading and calls the base implementation.
void TabContents::SetIsLoading(bool is_loading,
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/tab_contents/tab_contents.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -736,9 +736,13 @@ class TabContents : public PageNavigator,
// Shows a fade effect over this tab contents. Repeated calls will be ignored
// until the fade is canceled.
void FadeForInstant();

// Immediately removes the fade.
void CancelInstantFade();

// Opens view-source tab for this contents.
void ViewSource();

// Gets the minimum/maximum zoom percent.
int minimum_zoom_percent() const { return minimum_zoom_percent_; }
int maximum_zoom_percent() const { return maximum_zoom_percent_; }
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/tab_contents/tab_contents_delegate.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@ void TabContentsDelegate::ShowPageInfo(Profile* profile,
bool show_history) {
}

void TabContentsDelegate::ViewSourceForTab(TabContents* source) {
}

bool TabContentsDelegate::PreHandleKeyboardEvent(
const NativeWebKeyboardEvent& event,
bool* is_keyboard_shortcut) {
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/tab_contents/tab_contents_delegate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,9 @@ class TabContentsDelegate : public AutomationResourceRoutingDelegate {
const NavigationEntry::SSLStatus& ssl,
bool show_history);

// Opens source view for given tab contents.
virtual void ViewSourceForTab(TabContents* source);

// Allows delegates to handle keyboard events before sending to the renderer.
// Returns true if the |event| was handled. Otherwise, if the |event| would be
// handled in HandleKeyboardEvent() method as a normal keyboard shortcut,
Expand Down
132 changes: 78 additions & 54 deletions chrome/browser/ui/browser.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1526,15 +1526,8 @@ void Browser::SavePage() {
GetSelectedTabContents()->OnSavePage();
}

void Browser::ViewSource() {
UserMetrics::RecordAction(UserMetricsAction("ViewSource"), profile_);

TabContents* current_tab = GetSelectedTabContents();
NavigationEntry* entry = current_tab->controller().GetLastCommittedEntry();
if (entry) {
OpenURL(GURL(chrome::kViewSourceScheme + std::string(":") +
entry->url().spec()), GURL(), NEW_FOREGROUND_TAB, PageTransition::LINK);
}
void Browser::ViewSelectedSource() {
ViewSource(GetSelectedTabContentsWrapper());
}

void Browser::ShowFindBar() {
Expand Down Expand Up @@ -2152,7 +2145,7 @@ void Browser::ExecuteCommandWithDisposition(
case IDC_SAVE_PAGE: SavePage(); break;
case IDC_BOOKMARK_PAGE: BookmarkCurrentPage(); break;
case IDC_BOOKMARK_ALL_TABS: BookmarkAllTabs(); break;
case IDC_VIEW_SOURCE: ViewSource(); break;
case IDC_VIEW_SOURCE: ViewSelectedSource(); break;
case IDC_EMAIL_PAGE_LOCATION: EmailPageLocation(); break;
case IDC_PRINT: Print(); break;
case IDC_ENCODING_AUTO_DETECT: ToggleEncodingAutoDetect(); break;
Expand Down Expand Up @@ -2451,52 +2444,10 @@ bool Browser::CanDuplicateContentsAt(int index) {

void Browser::DuplicateContentsAt(int index) {
TabContentsWrapper* contents = GetTabContentsWrapperAt(index);
TabContents* new_contents = NULL;
DCHECK(contents);
bool pinned = false;

if (CanSupportWindowFeature(FEATURE_TABSTRIP)) {
// If this is a tabbed browser, just create a duplicate tab inside the same
// window next to the tab being duplicated.
TabContentsWrapper* wrapper = contents->Clone();
new_contents = wrapper->tab_contents();
pinned = tab_handler_->GetTabStripModel()->IsTabPinned(index);
int add_types = TabStripModel::ADD_SELECTED |
TabStripModel::ADD_INHERIT_GROUP |
(pinned ? TabStripModel::ADD_PINNED : 0);
tab_handler_->GetTabStripModel()->InsertTabContentsAt(index + 1,
wrapper,
add_types);
} else {
Browser* browser = NULL;
if (type_ & TYPE_APP) {
DCHECK((type_ & TYPE_POPUP) == 0);
DCHECK(type_ != TYPE_APP_PANEL);
browser = Browser::CreateForApp(app_name_, extension_app_, profile_,
false);
} else if (type_ == TYPE_POPUP) {
browser = Browser::CreateForType(TYPE_POPUP, profile_);
}

// Preserve the size of the original window. The new window has already
// been given an offset by the OS, so we shouldn't copy the old bounds.
BrowserWindow* new_window = browser->window();
new_window->SetBounds(gfx::Rect(new_window->GetRestoredBounds().origin(),
window()->GetRestoredBounds().size()));

// We need to show the browser now. Otherwise ContainerWin assumes the
// TabContents is invisible and won't size it.
browser->window()->Show();

// The page transition below is only for the purpose of inserting the tab.
new_contents = browser->AddTab(contents->Clone(), PageTransition::LINK);
}

if (profile_->HasSessionService()) {
SessionService* session_service = profile_->GetSessionService();
if (session_service)
session_service->TabRestored(&new_contents->controller(), pinned);
}
TabContentsWrapper* contents_dupe = contents->Clone();
InsertContentsDupe(contents, contents_dupe);
}

void Browser::CloseFrameAfterDragSession() {
Expand Down Expand Up @@ -3100,6 +3051,13 @@ void Browser::ShowPageInfo(Profile* profile,
window()->ShowPageInfo(profile, url, ssl, show_history);
}

void Browser::ViewSourceForTab(TabContents* contents) {
DCHECK(contents);
int index = tabstrip_model()->GetWrapperIndex(contents);
TabContentsWrapper* wrapper = tabstrip_model()->GetTabContentsAt(index);
ViewSource(wrapper);
}

bool Browser::PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event,
bool* is_keyboard_shortcut) {
return window()->PreHandleKeyboardEvent(event, is_keyboard_shortcut);
Expand Down Expand Up @@ -4166,3 +4124,69 @@ void Browser::CreateInstantIfNecessary() {
instant_unload_handler_.reset(new InstantUnloadHandler(this));
}
}

void Browser::ViewSource(TabContentsWrapper* contents) {
UserMetrics::RecordAction(UserMetricsAction("ViewSource"), profile_);
DCHECK(contents);

TabContentsWrapper* view_source_contents = contents->Clone();
view_source_contents->controller().PruneAllButActive();
NavigationEntry* active_entry =
view_source_contents->controller().GetActiveEntry();
GURL url = GURL(chrome::kViewSourceScheme + std::string(":") +
active_entry->url().spec());
active_entry->set_virtual_url(url);
InsertContentsDupe(contents, view_source_contents);
}

void Browser::InsertContentsDupe(
TabContentsWrapper* contents,
TabContentsWrapper* contents_dupe) {
DCHECK(contents);

TabContents* new_contents = contents_dupe->tab_contents();
bool pinned = false;

if (CanSupportWindowFeature(FEATURE_TABSTRIP)) {
// If this is a tabbed browser, just create a duplicate tab inside the same
// window next to the tab being duplicated.
int index = tab_handler_->GetTabStripModel()->
GetIndexOfTabContents(contents);
pinned = tab_handler_->GetTabStripModel()->IsTabPinned(index);
int add_types = TabStripModel::ADD_SELECTED |
TabStripModel::ADD_INHERIT_GROUP |
(pinned ? TabStripModel::ADD_PINNED : 0);
tab_handler_->GetTabStripModel()->InsertTabContentsAt(index + 1,
contents_dupe,
add_types);
} else {
Browser* browser = NULL;
if (type_ & TYPE_APP) {
DCHECK((type_ & TYPE_POPUP) == 0);
DCHECK(type_ != TYPE_APP_PANEL);
browser = Browser::CreateForApp(app_name_, extension_app_, profile_,
false);
} else if (type_ == TYPE_POPUP) {
browser = Browser::CreateForType(TYPE_POPUP, profile_);
}

// Preserve the size of the original window. The new window has already
// been given an offset by the OS, so we shouldn't copy the old bounds.
BrowserWindow* new_window = browser->window();
new_window->SetBounds(gfx::Rect(new_window->GetRestoredBounds().origin(),
window()->GetRestoredBounds().size()));

// We need to show the browser now. Otherwise ContainerWin assumes the
// TabContents is invisible and won't size it.
browser->window()->Show();

// The page transition below is only for the purpose of inserting the tab.
browser->AddTab(contents_dupe, PageTransition::LINK);
}

if (profile_->HasSessionService()) {
SessionService* session_service = profile_->GetSessionService();
if (session_service)
session_service->TabRestored(&new_contents->controller(), pinned);
}
}
12 changes: 11 additions & 1 deletion chrome/browser/ui/browser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ class Browser : public TabHandlerDelegate,
// Page-related commands
void BookmarkCurrentPage();
void SavePage();
void ViewSource();
void ViewSelectedSource();
void ShowFindBar();

// Returns true if the Browser supports the specified feature. The value of
Expand Down Expand Up @@ -778,6 +778,7 @@ class Browser : public TabHandlerDelegate,
const GURL& url,
const NavigationEntry::SSLStatus& ssl,
bool show_history);
virtual void ViewSourceForTab(TabContents* source);
virtual bool PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event,
bool* is_keyboard_shortcut);
virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event);
Expand Down Expand Up @@ -986,6 +987,15 @@ class Browser : public TabHandlerDelegate,
// If this browser should have instant one is created, otherwise does nothing.
void CreateInstantIfNecessary();

// Opens view-source tab for given tab contents.
void ViewSource(TabContentsWrapper* tab);

// Inserts contents dupe next to the original contents. This method is used
// to insert duplicate tab and view source tab next to the original tab.
void InsertContentsDupe(
TabContentsWrapper* original_content,
TabContentsWrapper* clone_content);

// Data members /////////////////////////////////////////////////////////////

NotificationRegistrar registrar_;
Expand Down

0 comments on commit 77d8d62

Please sign in to comment.