Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(171)

Issue 67201: Fix the process creation problem. This forces transitions between... (Closed)

Created:
11 years, 8 months ago by brettw
Modified:
9 years, 7 months ago
Reviewers:
Charlie Reis
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Fix the process creation problem. This forces transitions between BrowsingInstances when we force a transition for DOM UIs. BUG=9364 TEST=see bug Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=13892

Patch Set 1 #

Total comments: 1

Patch Set 2 : '' #

Patch Set 3 : '' #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+68 lines, -5 lines) Patch
M chrome/browser/tab_contents/render_view_host_manager.h View 1 1 chunk +5 lines, -3 lines 1 comment Download
M chrome/browser/tab_contents/render_view_host_manager.cc View 1 3 chunks +9 lines, -2 lines 1 comment Download
A chrome/browser/tab_contents/render_view_host_manager_unittest.cc View 2 1 chunk +49 lines, -0 lines 2 comments Download
M chrome/chrome.gyp View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/test/unit/unittests.vcproj View 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
brettw
11 years, 8 months ago (2009-04-16 04:29:34 UTC) #1
Charlie Reis
I think this is probably the easiest way to fix the major problem we have ...
11 years, 8 months ago (2009-04-16 05:57:43 UTC) #2
brettw
New snap up. This includes a unittest.
11 years, 8 months ago (2009-04-16 21:35:53 UTC) #3
Charlie Reis
11 years, 8 months ago (2009-04-16 22:00:56 UTC) #4
LGTM-- just a few nits on comments.  Thanks!

http://codereview.chromium.org/67201/diff/11/13
File chrome/browser/tab_contents/render_view_host_manager.cc (right):

http://codereview.chromium.org/67201/diff/11/13#newcode385
Line 385: // BrowsingInstance.
I had trouble following the last sentence.  Perhaps this?

This addresses special cases where we use a single BrowsingInstance for all
pages of a certain type (e.g., New Tab Pages), keeping them in the same process.

http://codereview.chromium.org/67201/diff/11/12
File chrome/browser/tab_contents/render_view_host_manager.h (right):

http://codereview.chromium.org/67201/diff/11/12#newcode191
Line 191: // be the same. It also forces new SiteInstances for these
transitions.
"new SiteInstances and BrowsingInstances for these transitions."

Maybe also point out examples of when this is this case?  DOM UI pages, and soon
extensions (according to Matt).

http://codereview.chromium.org/67201/diff/11/15
File chrome/browser/tab_contents/render_view_host_manager_unittest.cc (right):

http://codereview.chromium.org/67201/diff/11/15#newcode14
Line 14: // then do that sam ething in another tab, that the two resulting pages
have
Typo: same thing

http://codereview.chromium.org/67201/diff/11/15#newcode31
Line 31: // Load the two URLs in the second tab. Note that the first situation
create
"situation create" -> "navigation creates"
(I was confused thinking that "situation" referred to the navigations in the
first tab.)

Powered by Google App Engine
This is Rietveld 408576698