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

Issue 76063007: Clean up RenderViewHostManager swapping logic. (Closed)

Created:
7 years, 1 month ago by Charlie Reis
Modified:
7 years ago
Reviewers:
nasko
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam
Visibility:
Public.

Description

Clean up RenderViewHostManager swapping logic. Makes the difference between swapping SiteInstances and swapping BrowsingInstances explicit. BUG=123007 TEST=No functionality change. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=237388

Patch Set 1 : Initial patch #

Patch Set 2 : Try updating CCBC::ShouldSwap #

Patch Set 3 : Rebase #

Patch Set 4 : Defer CCBC changes to another CL #

Total comments: 4

Patch Set 5 : Fix nits #

Unified diffs Side-by-side diffs Delta from patch set Stats (+127 lines, -111 lines) Patch
M chrome/browser/chrome_content_browser_client.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chrome_content_browser_client.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M content/browser/frame_host/render_view_host_manager.h View 1 chunk +13 lines, -11 lines 0 comments Download
M content/browser/frame_host/render_view_host_manager.cc View 1 2 3 4 15 chunks +100 lines, -90 lines 0 comments Download
M content/browser/frame_host/render_view_host_manager_unittest.cc View 1 chunk +3 lines, -2 lines 0 comments Download
M content/public/browser/content_browser_client.h View 1 2 1 chunk +8 lines, -5 lines 0 comments Download
M content/public/browser/content_browser_client.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 5 (0 generated)
Charlie Reis
Nasko, here's more cleanup I've been hoping to do to RenderViewHostManager. There isn't much behavior ...
7 years ago (2013-11-26 05:49:53 UTC) #1
nasko
LGTM with couple of nits. https://codereview.chromium.org/76063007/diff/330001/content/browser/frame_host/render_view_host_manager.cc File content/browser/frame_host/render_view_host_manager.cc (right): https://codereview.chromium.org/76063007/diff/330001/content/browser/frame_host/render_view_host_manager.cc#newcode548 content/browser/frame_host/render_view_host_manager.cc:548: // "view-source:http://foo.com/" and "http://foo.com/", ...
7 years ago (2013-11-26 15:06:06 UTC) #2
Charlie Reis
Thanks! https://codereview.chromium.org/76063007/diff/330001/content/browser/frame_host/render_view_host_manager.cc File content/browser/frame_host/render_view_host_manager.cc (right): https://codereview.chromium.org/76063007/diff/330001/content/browser/frame_host/render_view_host_manager.cc#newcode548 content/browser/frame_host/render_view_host_manager.cc:548: // "view-source:http://foo.com/" and "http://foo.com/", WebKit doesn't treat On ...
7 years ago (2013-11-26 17:44:49 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/creis@chromium.org/76063007/350001
7 years ago (2013-11-26 17:45:37 UTC) #4
commit-bot: I haz the power
7 years ago (2013-11-26 19:43:04 UTC) #5
Message was sent while issue was closed.
Change committed as 237388

Powered by Google App Engine
This is Rietveld 408576698