Issue 11416042: views: Change the LocationBarView delegate's API to GetWebContents().

Issue 11416042: views: Change the LocationBarView delegate's API to GetWebContents(). (Closed)

Created:
8 years, 1 month ago by tfarina

Modified:
8 years, 1 month ago

Reviewers:
Peter Kasting

CC:
chromium-reviews, tfarina, oshima+watch_chromium.org, stevenjb+watch_chromium.org, nkostylev+watch_chromium.org, davemoore+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

views: Change the LocationBarView delegate's API to GetWebContents(). BUG=107201 R=pkasting@chromium.org

Patch Set 2 : fixed #

Total comments: 2

Created: 8 years, 1 month ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+19 lines, -29 lines)			Patch
M	chrome/browser/chromeos/login/simple_web_view_dialog.h	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/chromeos/login/simple_web_view_dialog.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/ui/views/location_bar/location_bar_view.h	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/ui/views/location_bar/location_bar_view.cc	View	1	9 chunks	+12 lines, -22 lines	2 comments	Download
M	chrome/browser/ui/views/location_bar/zoom_view.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/ui/views/toolbar_view.h	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/ui/views/toolbar_view.cc	View		1 chunk	+2 lines, -2 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

tfarina

http://codereview.chromium.org/11416042/diff/3001/chrome/browser/ui/views/location_bar/location_bar_view.cc File chrome/browser/ui/views/location_bar/location_bar_view.cc (right): http://codereview.chromium.org/11416042/diff/3001/chrome/browser/ui/views/location_bar/location_bar_view.cc#newcode1145 chrome/browser/ui/views/location_bar/location_bar_view.cc:1145: return TabContents::FromWebContents(delegate_->GetWebContents()); This is segfaulting. I'm looking into this ...

8 years, 1 month ago (2012-11-16 21:42:18 UTC) #2

tfarina

On Fri, Nov 16, 2012 at 7:42 PM, <tfarina@chromium.org> wrote: > http://codereview.chromium.org/11416042/diff/3001/chrome/browser/ui/views/location_bar/location_bar_view.cc#newcode1145 > chrome/browser/ui/views/location_bar/location_bar_view.cc:1145: return ...

8 years, 1 month ago (2012-11-16 22:03:08 UTC) #3

Peter Kasting

8 years, 1 month ago (2012-11-16 23:12:13 UTC) #4

LGTM

https://codereview.chromium.org/11416042/diff/7001/chrome/browser/ui/views/lo...
File chrome/browser/ui/views/location_bar/location_bar_view.cc (right):

https://codereview.chromium.org/11416042/diff/7001/chrome/browser/ui/views/lo...
chrome/browser/ui/views/location_bar/location_bar_view.cc:601:
browser::FindBrowserWithWebContents(delegate_->GetWebContents()));
Nit: Move "action_box_button_view_," to a new 4-space indented line as well, so
lines of arguments are aligned.

(My interpretation of the Google style guide has changed after discussion with
the internal style mailing lists.)

https://codereview.chromium.org/11416042/diff/7001/chrome/browser/ui/views/lo...
chrome/browser/ui/views/location_bar/location_bar_view.cc:1270: Browser* browser
= browser::FindBrowserWithWebContents(
Tiny nit: Prefer to linebreak after '='

Expand Messages | Collapse Messages