Issue 174021: Convert users of the "get last active browser, get selected tab contents, ope...

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Issue 174021: Convert users of the "get last active browser, get selected tab contents, ope... (Closed)

Created:
11 years, 4 months ago by Ben Goodger (Google)

Modified:
9 years, 6 months ago

Reviewers:
John Grabowski

CC:
chromium-reviews_googlegroups.com, John Grabowski, Ben Goodger (Google)

Base URL:
svn://chrome-svn/chrome/trunk/src/

Visibility:
Public.

Description

Convert users of the "get last active browser, get selected tab contents, open url" pattern to just call OpenURL on Browser directly. Makes GetOrCreateTabbedBrowser public on Browser, and makes it static so it can be called with a provided profile. BUG=none TEST=Try opening links from the bookmark/history menus on mac, with and without an existing window open, with an active incognito window, etc. The links should all open in the last active non-incognito window, or create a new non-incognito window if none is open.

Patch Set 1 #

Patch Set 2 : '' #

Created: 11 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+25 lines, -47 lines)			Patch
M	chrome/browser/browser.h	View	1	2 chunks	+3 lines, -4 lines	0 comments	Download
M	chrome/browser/browser.cc	View	1	5 chunks	+8 lines, -7 lines	0 comments	Download
M	chrome/browser/cocoa/bookmark_menu_cocoa_controller.mm	View	1	2 chunks	+7 lines, -16 lines	0 comments	Download
M	chrome/browser/cocoa/browser_window_controller.mm	View	1	1 chunk	+1 line, -4 lines	0 comments	Download
M	chrome/browser/cocoa/history_menu_cocoa_controller.mm	View	1	2 chunks	+6 lines, -16 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

John Grabowski

LGTM Lame that browser_unittest.cc has only one test... which is commented out. Since you're right ...

11 years, 4 months ago (2009-08-18 21:48:06 UTC) #2

Ben Goodger (Google)

11 years, 4 months ago (2009-08-18 22:24:20 UTC) #3

Going to get the browser_unittest re-enabled in a separate CL since I
can't currently verify this on Mac.

-Ben

On Tue, Aug 18, 2009 at 2:48 PM, <jrg@chromium.org> wrote:
> LGTM
>
> Lame that browser_unittest.cc has only one test... which is commented
> out. =A0Since you're right here, please spend a minute or two to clean
> that up. =A0Perhaps delete the file, or add a comment explaining that the
> in-process browser tests are the effective unit test?
>
>
> http://codereview.chromium.org/174021
>

Expand Messages | Collapse Messages