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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 10554010: Remove BrowserList::GetLastActive usage from WebUI pages. (Closed)

Created:
8 years, 6 months ago by jam
Modified:
8 years, 6 months ago
CC:
chromium-reviews
Visibility:
Public.

Description

Remove BrowserList::GetLastActive usage from WebUI pages. I've plumbed the TabContents to the page bubble code so that the webui certificate viewer can get the Browser from it. BUG=129187 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=142963

Patch Set 1 #

Total comments: 1

Patch Set 2 : #

Patch Set 3 : #

Total comments: 2

Patch Set 4 : WebContents instead of TabContents #

Patch Set 5 : fix mac #

Patch Set 6 : #

Patch Set 7 : mac build #

Total comments: 4

Patch Set 8 : review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+124 lines, -83 lines) Patch
M chrome/browser/certificate_viewer.h View 1 2 3 2 chunks +9 lines, -2 lines 0 comments Download
M chrome/browser/certificate_viewer.cc View 1 2 3 2 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/login/simple_web_view_dialog.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/tab_contents/tab_contents_ssl_helper.cc View 1 2 3 1 chunk +3 lines, -1 line 0 comments Download
M chrome/browser/ui/android/certificate_viewer_android.cc View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/browser.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/browser_list.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/ui/browser_window.h View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/cocoa/browser_window_cocoa.h View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/cocoa/browser_window_cocoa.mm View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/cocoa/certificate_viewer_mac.mm View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/cocoa/page_info_bubble_controller.h View 1 2 3 4 5 3 chunks +5 lines, -0 lines 0 comments Download
M chrome/browser/ui/cocoa/page_info_bubble_controller.mm View 1 2 3 4 5 6 7 6 chunks +10 lines, -4 lines 0 comments Download
M chrome/browser/ui/cocoa/page_info_bubble_controller_unittest.mm View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/gtk/browser_window_gtk.h View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/gtk/browser_window_gtk.cc View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/gtk/certificate_viewer_gtk.cc View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/gtk/page_info_bubble_gtk.cc View 1 2 3 4 5 6 7 8 chunks +16 lines, -11 lines 0 comments Download
M chrome/browser/ui/gtk/ssl_client_certificate_selector.cc View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/gtk/website_settings_popup_gtk.cc View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/page_info_bubble.h View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/panels/panel_browser_window.h View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/panels/panel_browser_window.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/browser_dialogs.h View 1 2 3 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/ui/views/certificate_viewer_win.cc View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/views/frame/browser_view.h View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/frame/browser_view.cc View 1 2 3 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/page_info_bubble_view.h View 1 2 3 3 chunks +4 lines, -1 line 0 comments Download
M chrome/browser/ui/views/page_info_bubble_view.cc View 1 2 3 4 5 6 7 5 chunks +12 lines, -8 lines 0 comments Download
M chrome/browser/ui/views/ssl_client_certificate_selector.cc View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/views/stubs_aura.cc View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/webui/certificate_viewer_webui.h View 1 2 3 2 chunks +5 lines, -6 lines 0 comments Download
M chrome/browser/ui/webui/certificate_viewer_webui.cc View 1 2 3 4 chunks +9 lines, -11 lines 0 comments Download
M chrome/browser/ui/webui/feedback_ui.cc View 2 chunks +3 lines, -4 lines 0 comments Download
M chrome/browser/ui/webui/options2/certificate_manager_handler2.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/test/base/test_browser_window.h View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/test/data/webui/certificate_viewer_ui_test-inl.h View 1 2 3 4 5 1 chunk +2 lines, -1 line 0 comments Download
M ui/web_dialogs/constrained_web_dialog_ui.h View 1 2 3 4 5 6 7 3 chunks +5 lines, -4 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
jam
8 years, 6 months ago (2012-06-15 20:01:37 UTC) #1
Ben Goodger (Google)
http://codereview.chromium.org/10554010/diff/1/chrome/browser/ui/webui/certificate_viewer_webui.cc File chrome/browser/ui/webui/certificate_viewer_webui.cc (right): http://codereview.chromium.org/10554010/diff/1/chrome/browser/ui/webui/certificate_viewer_webui.cc#newcode73 chrome/browser/ui/webui/certificate_viewer_webui.cc:73: browser::FindBrowserWithWebContents(web_ui()->GetWebContents()); so, this method is pretty much just as ...
8 years, 6 months ago (2012-06-15 20:15:44 UTC) #2
jam
ptal
8 years, 6 months ago (2012-06-18 20:09:52 UTC) #3
Ben Goodger (Google)
http://codereview.chromium.org/10554010/diff/20001/chrome/browser/ui/page_info_bubble.h File chrome/browser/ui/page_info_bubble.h (right): http://codereview.chromium.org/10554010/diff/20001/chrome/browser/ui/page_info_bubble.h#newcode22 chrome/browser/ui/page_info_bubble.h:22: TabContents* tab_contents, why not WebContents?
8 years, 6 months ago (2012-06-18 20:26:33 UTC) #4
jam
http://codereview.chromium.org/10554010/diff/20001/chrome/browser/ui/page_info_bubble.h File chrome/browser/ui/page_info_bubble.h (right): http://codereview.chromium.org/10554010/diff/20001/chrome/browser/ui/page_info_bubble.h#newcode22 chrome/browser/ui/page_info_bubble.h:22: TabContents* tab_contents, On 2012/06/18 20:26:33, Ben Goodger (Google) wrote: ...
8 years, 6 months ago (2012-06-18 20:33:12 UTC) #5
jam
ptal
8 years, 6 months ago (2012-06-19 01:03:59 UTC) #6
Ben Goodger (Google)
lgtm http://codereview.chromium.org/10554010/diff/16019/chrome/browser/ui/views/page_info_bubble_view.cc File chrome/browser/ui/views/page_info_bubble_view.cc (right): http://codereview.chromium.org/10554010/diff/16019/chrome/browser/ui/views/page_info_bubble_view.cc#newcode114 chrome/browser/ui/views/page_info_bubble_view.cc:114: TabContents::FromWebContents(web_contents)->profile(), url, ssl, Profile::FromBrowserContext(web_contents->GetBrowserContext()) avoids the TC include ...
8 years, 6 months ago (2012-06-19 14:55:07 UTC) #7
jam
8 years, 6 months ago (2012-06-19 15:13:28 UTC) #8 
http://codereview.chromium.org/10554010/diff/16019/chrome/browser/ui/views/pa...
File chrome/browser/ui/views/page_info_bubble_view.cc (right):

http://codereview.chromium.org/10554010/diff/16019/chrome/browser/ui/views/pa...
chrome/browser/ui/views/page_info_bubble_view.cc:114:
TabContents::FromWebContents(web_contents)->profile(), url, ssl,
On 2012/06/19 14:55:07, Ben Goodger (Google) wrote:
> Profile::FromBrowserContext(web_contents->GetBrowserContext()) avoids the TC
> include

Done.

http://codereview.chromium.org/10554010/diff/16019/ui/web_dialogs/constrained...
File ui/web_dialogs/constrained_web_dialog_ui.h (right):

http://codereview.chromium.org/10554010/diff/16019/ui/web_dialogs/constrained...
ui/web_dialogs/constrained_web_dialog_ui.h:44: virtual ConstrainedWindow*
window() = 0;
On 2012/06/19 14:55:07, Ben Goodger (Google) wrote:
> can you add a TODO here to fix these function names

Done.

Powered by Google App Engine
This is Rietveld 408576698