diff options
| author | twiz@google.com <twiz@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-09-23 22:45:25 +0000 |
|---|---|---|
| committer | twiz@google.com <twiz@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-09-23 22:45:25 +0000 |
| commit | 088ee1afadb121aaf01cb033c373ade1d35456ac (patch) | |
| tree | 0a303393f9850c6916d6314c1ed9fd109b2fe137 | |
| parent | dec76e804867ced17b032571f440adf4945c4d99 (diff) | |
| download | chromium_src-088ee1afadb121aaf01cb033c373ade1d35456ac.zip chromium_src-088ee1afadb121aaf01cb033c373ade1d35456ac.tar.gz chromium_src-088ee1afadb121aaf01cb033c373ade1d35456ac.tar.bz2 | |
Simple fix correcting the assignment of page_id to RenderView instances.
The logic in NavigationController::ClassifyNavigation assumes that all new pages will have an id LARGER than the maximal id that is locally cached in the SiteInstance of the active RenderViewHost.
Because page ids are doled out by incrementing a static variable that resides in the render process - see RenderView::next_page_id_ - resetting this counter while the render process is still in use will confuse the browser, and trigger the assert logic in NavigationController::ClassifyNavigation. Newly created RenderView objects in this render process will send page ids less than the value stored in SiteInstance::max_page_id during navigation operations.
This problem was manifested during the interaction of multiple instances of a chrome-extension residing in ActiveX controls. Consider navigating a Chrome-Frame instance to an extension page. Now navigate a second Chrome-Frame instance to another extension page within the same extension. If a top-level link navigation is initiated on the second CF instance, then the extension will be torn down, but the old logic would have reset the next_page_id_ counter. If a subsequent CF was navigated to a page within the same extension, it would be backed by the same extension process, yet it would be assigned an incorrect page id, as described above.
BUG=55138
TEST=none
Review URL: http://codereview.chromium.org/3442007
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@60370 0039d316-1c4b-4281-b951-d872f2087c98
| -rw-r--r-- | chrome/browser/tab_contents/navigation_controller.cc | 6 | ||||
| -rw-r--r-- | chrome/renderer/render_view.cc | 1 |
2 files changed, 0 insertions, 7 deletions
diff --git a/chrome/browser/tab_contents/navigation_controller.cc b/chrome/browser/tab_contents/navigation_controller.cc index 819bf6e..b54c394 100644 --- a/chrome/browser/tab_contents/navigation_controller.cc +++ b/chrome/browser/tab_contents/navigation_controller.cc @@ -639,12 +639,6 @@ NavigationType::Type NavigationController::ClassifyNavigation( tab_contents_->GetSiteInstance(), params.page_id); if (existing_entry_index == -1) { - // TODO(twiz) Top-level, out-of-browser navigations from ActiveX instances - // of Chrome Frame can trigger this behaviour: The page_id is less than - // GetMaxPageID, yet no entry index is registered. See BUG 55138. - if (PageTransition::IsMainFrame(params.transition)) - return NavigationType::NEW_PAGE; - // The page was not found. It could have been pruned because of the limit on // back/forward entries (not likely since we'll usually tell it to navigate // to such entries). It could also mean that the renderer is smoking crack. diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc index 037a275..9666b67 100644 --- a/chrome/renderer/render_view.cc +++ b/chrome/renderer/render_view.cc @@ -2587,7 +2587,6 @@ WebNavigationPolicy RenderView::decidePolicyForNavigation( // Reset these counters as the RenderView could be reused for the next // navigation. page_id_ = -1; - next_page_id_ = 1; last_page_id_sent_to_browser_ = -1; OpenURL(url, referrer, default_policy); return WebKit::WebNavigationPolicyIgnore; // Suppress the load here. |