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

Issue 8224023: Don't show URL for pending new navigations initiated by the renderer. (Closed)

Created:
9 years, 2 months ago by Charlie Reis
Modified:
9 years, 2 months ago
CC:
chromium-reviews, Avi (use Gerrit), creis+watch_chromium.org, dpranke+watch-content_chromium.org, jam, joi+watch-content_chromium.org, darin-cc_chromium.org, brettw-cc_chromium.org, ajwong+watch_chromium.org, Aaron Boodman
Visibility:
Public.

Description

Don't show URL for pending new navigations initiated by the renderer. BUG=99016 TEST=Click a link to a slow view-source: URL. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=105355

Patch Set 1 #

Total comments: 4

Patch Set 2 : Add IsBrowserInitiated helper. #

Total comments: 6

Patch Set 3 : Add is_renderer_initiated #

Patch Set 4 : Update navigation_entry_unittest #

Patch Set 5 : Fix test compile. #

Patch Set 6 : Fix more OpenURLParams call sites. #

Patch Set 7 : Fix additional tests. #

Patch Set 8 : Fix merge conflicts. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+218 lines, -65 lines) Patch
M chrome/browser/bookmarks/bookmark_context_menu_controller_unittest.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/bookmarks/bookmark_utils.cc View 1 2 3 4 5 6 7 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/login/registration_screen.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/debugger/devtools_window.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/extensions/webstore_inline_installer.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/external_tab_container_win.cc View 1 2 3 4 5 6 7 2 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/oom_priority_manager_browsertest.cc View 1 2 3 4 5 6 7 1 chunk +5 lines, -3 lines 0 comments Download
M chrome/browser/rlz/rlz_unittest.cc View 1 2 3 4 5 6 7 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/sessions/session_types.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/sync/test/integration/performance/sessions_sync_perf_test.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/blocked_content/blocked_content_container.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/browser.cc View 1 2 3 4 5 6 7 3 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/ui/browser_navigator.h View 1 2 3 4 5 6 7 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/ui/browser_navigator.cc View 1 2 3 4 5 6 7 2 chunks +16 lines, -5 lines 0 comments Download
M chrome/browser/ui/gtk/about_chrome_dialog.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/gtk/bookmarks/bookmark_menu_controller_gtk.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/gtk/extensions/extension_install_dialog_gtk.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/gtk/global_bookmark_menu.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/gtk/global_history_menu.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/gtk/location_bar_view_gtk.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/gtk/page_info_bubble_gtk.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/gtk/sad_tab_gtk.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/gtk/tabs/dragged_tab_controller_gtk.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/gtk/view_id_util_browsertest.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/views/bookmarks/bookmark_bar_view_test.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/views/bookmarks/bookmark_context_menu_test.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/views/tabs/dragged_tab_controller.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/webui/active_downloads_ui.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/webui/html_dialog_tab_contents_delegate.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download
M content/browser/site_instance_unittest.cc View 1 2 3 4 5 6 7 3 chunks +6 lines, -3 lines 0 comments Download
M content/browser/tab_contents/navigation_controller.h View 1 2 3 4 5 6 7 2 chunks +8 lines, -0 lines 0 comments Download
M content/browser/tab_contents/navigation_controller.cc View 1 2 3 4 5 6 7 5 chunks +33 lines, -4 lines 0 comments Download
M content/browser/tab_contents/navigation_controller_unittest.cc View 1 2 3 4 5 6 7 3 chunks +40 lines, -2 lines 0 comments Download
M content/browser/tab_contents/navigation_entry.h View 1 2 3 4 5 6 7 3 chunks +16 lines, -1 line 0 comments Download
M content/browser/tab_contents/navigation_entry.cc View 1 2 3 4 5 6 7 3 chunks +6 lines, -3 lines 0 comments Download
M content/browser/tab_contents/navigation_entry_unittest.cc View 1 2 3 4 5 6 7 2 chunks +8 lines, -1 line 0 comments Download
M content/browser/tab_contents/page_navigator.h View 1 2 3 4 5 6 7 2 chunks +5 lines, -1 line 0 comments Download
M content/browser/tab_contents/page_navigator.cc View 1 2 3 4 5 6 7 1 chunk +6 lines, -3 lines 0 comments Download
M content/browser/tab_contents/render_view_host_manager_unittest.cc View 1 2 3 4 5 6 7 6 chunks +12 lines, -6 lines 0 comments Download
M content/browser/tab_contents/tab_contents.cc View 1 2 3 4 5 6 7 2 chunks +5 lines, -3 lines 0 comments Download
M content/browser/tab_contents/tab_contents_delegate.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 25 (0 generated)
Charlie Reis
Brett, can you review? Aaron, I'm cc'ing you as an FYI for the ShouldDisplayURL change ...
9 years, 2 months ago (2011-10-11 00:55:05 UTC) #1
brettw
I thought we didn't have pending entries for renderer-initiated transitions. Did something change?
9 years, 2 months ago (2011-10-11 03:38:10 UTC) #2
Charlie Reis
On 2011/10/11 03:38:10, brettw wrote: > I thought we didn't have pending entries for renderer-initiated ...
9 years, 2 months ago (2011-10-11 14:32:57 UTC) #3
brettw
http://codereview.chromium.org/8224023/diff/1/content/browser/tab_contents/navigation_controller.cc File content/browser/tab_contents/navigation_controller.cc (right): http://codereview.chromium.org/8224023/diff/1/content/browser/tab_contents/navigation_controller.cc#newcode300 content/browser/tab_contents/navigation_controller.cc:300: pending_entry_->transition_type() != PageTransition::LINK) I'm kind of freaked out by ...
9 years, 2 months ago (2011-10-11 18:11:29 UTC) #4
Charlie Reis
http://codereview.chromium.org/8224023/diff/1/content/browser/tab_contents/navigation_controller.cc File content/browser/tab_contents/navigation_controller.cc (right): http://codereview.chromium.org/8224023/diff/1/content/browser/tab_contents/navigation_controller.cc#newcode300 content/browser/tab_contents/navigation_controller.cc:300: pending_entry_->transition_type() != PageTransition::LINK) On 2011/10/11 18:11:29, brettw wrote: > ...
9 years, 2 months ago (2011-10-11 19:40:42 UTC) #5
brettw
http://codereview.chromium.org/8224023/diff/1/content/browser/tab_contents/navigation_controller.cc File content/browser/tab_contents/navigation_controller.cc (right): http://codereview.chromium.org/8224023/diff/1/content/browser/tab_contents/navigation_controller.cc#newcode300 content/browser/tab_contents/navigation_controller.cc:300: pending_entry_->transition_type() != PageTransition::LINK) I don't think we ever expected ...
9 years, 2 months ago (2011-10-11 19:51:09 UTC) #6
Charlie Reis
http://codereview.chromium.org/8224023/diff/1/content/browser/tab_contents/navigation_controller.cc File content/browser/tab_contents/navigation_controller.cc (right): http://codereview.chromium.org/8224023/diff/1/content/browser/tab_contents/navigation_controller.cc#newcode300 content/browser/tab_contents/navigation_controller.cc:300: pending_entry_->transition_type() != PageTransition::LINK) On 2011/10/11 19:51:09, brettw wrote: > ...
9 years, 2 months ago (2011-10-11 20:14:29 UTC) #7
Charlie Reis
On 2011/10/11 20:14:29, creis wrote: > Well, if we wanted to approach this another way, ...
9 years, 2 months ago (2011-10-11 21:24:17 UTC) #8
Paweł Hajdan Jr.
Drive-by with minor nits, no need to wait for me if you fix them. http://codereview.chromium.org/8224023/diff/6001/chrome/browser/ui/toolbar/toolbar_model_unittest.cc ...
9 years, 2 months ago (2011-10-11 22:26:18 UTC) #9
Charlie Reis
On 2011/10/11 22:26:18, Paweł Hajdan Jr. wrote: > Drive-by with minor nits, no need to ...
9 years, 2 months ago (2011-10-11 22:49:44 UTC) #10
brettw
This still contains the code from the split off review. Is this on purpose, should ...
9 years, 2 months ago (2011-10-12 17:45:15 UTC) #11
Charlie Reis
On 2011/10/12 17:45:15, brettw wrote: > This still contains the code from the split off ...
9 years, 2 months ago (2011-10-12 17:47:56 UTC) #12
brettw
I'm surprised we need to change LoadURL. Wouldn't all of browser-initiated loads just implicitly set ...
9 years, 2 months ago (2011-10-12 17:50:58 UTC) #13
Charlie Reis
On 2011/10/12 17:50:58, brettw wrote: > I'm surprised we need to change LoadURL. Wouldn't all ...
9 years, 2 months ago (2011-10-12 18:06:26 UTC) #14
brettw
Does this call need to go to the browser? I would have expected that cross-site ...
9 years, 2 months ago (2011-10-12 18:09:45 UTC) #15
Charlie Reis
On 2011/10/12 18:09:45, brettw wrote: > Does this call need to go to the browser? ...
9 years, 2 months ago (2011-10-12 18:19:22 UTC) #16
brettw
I agree, I think we just re-used the new tab code path when we added ...
9 years, 2 months ago (2011-10-12 19:21:57 UTC) #17
Charlie Reis
On 2011/10/12 19:21:57, brettw wrote: > I agree, I think we just re-used the new ...
9 years, 2 months ago (2011-10-12 19:58:48 UTC) #18
Charlie Reis
Albert suggests passing the new parameter up until LoadURL (which is where the large fanout ...
9 years, 2 months ago (2011-10-12 20:17:43 UTC) #19
brettw
Okay. I feel like maybe we need a LoadURLParams structure that has some reasonable defaults ...
9 years, 2 months ago (2011-10-12 21:00:30 UTC) #20
Charlie Reis
Ok, I've put some explicit is_renderer_initiated state on NavigationEntry and threaded it through the call ...
9 years, 2 months ago (2011-10-12 21:55:56 UTC) #21
brettw
lgtm
9 years, 2 months ago (2011-10-12 22:41:20 UTC) #22
Charlie Reis
Drew, can you take a look at the sessions_sync_perf_test.cc change for owners approval? Brett, just ...
9 years, 2 months ago (2011-10-13 17:24:04 UTC) #23
Andrew T Wilson (Slow)
LGTM
9 years, 2 months ago (2011-10-13 18:37:41 UTC) #24
Timur Iskhodzhanov
9 years, 2 months ago (2011-10-14 09:33:08 UTC) #25
This CL has introduced http://crbug/100315

Powered by Google App Engine
This is Rietveld 408576698