Issue 11087022: intents: Change default_service_url() to return a const-ref.

Issue 11087022: intents: Change default_service_url() to return a const-ref. (Closed)

Created:
8 years, 2 months ago by tfarina

Modified:
8 years, 2 months ago

Reviewers:
groby-ooo-7-16

CC:
chromium-reviews, gbillock+watch_chromium.org, smckay+watch_chromium.org, groby+watch_chromium.org, rouslan+watch_chromium.org

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

Visibility:
Public.

More Reviews

Description

intents: Change default_service_url() to return a const-ref. Apply similar fixes to web_intents_model.h R=groby@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=160957

Patch Set 4 : indent 4 spaces #

Patch Set 5 : rm virtual dtor #

Created: 8 years, 2 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+33 lines, -24 lines)			Patch
M	chrome/browser/ui/intents/web_intent_picker_model.h	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/ui/intents/web_intents_model.h	View	1 2 3 4	7 chunks	+23 lines, -14 lines	0 comments	Download
M	chrome/browser/ui/intents/web_intents_model.cc	View	1 2 3	2 chunks	+9 lines, -9 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

tfarina

Hi Rachel, could you review this to me? http://codereview.chromium.org/11087022/diff/1/chrome/browser/ui/intents/web_intents_model.h File chrome/browser/ui/intents/web_intents_model.h (right): http://codereview.chromium.org/11087022/diff/1/chrome/browser/ui/intents/web_intents_model.h#newcode52 chrome/browser/ui/intents/web_intents_model.h:52: const ...

8 years, 2 months ago (2012-10-09 01:43:54 UTC) #1

groby-ooo-7-16

lgtm http://codereview.chromium.org/11087022/diff/2001/chrome/browser/ui/intents/web_intents_model.h File chrome/browser/ui/intents/web_intents_model.h (right): http://codereview.chromium.org/11087022/diff/2001/chrome/browser/ui/intents/web_intents_model.h#newcode90 chrome/browser/ui/intents/web_intents_model.h:90: virtual ~Observer() {} nit: Technically not needed, since ...

8 years, 2 months ago (2012-10-09 01:58:22 UTC) #2

tfarina

http://codereview.chromium.org/11087022/diff/2001/chrome/browser/ui/intents/web_intents_model.h File chrome/browser/ui/intents/web_intents_model.h (right): http://codereview.chromium.org/11087022/diff/2001/chrome/browser/ui/intents/web_intents_model.h#newcode90 chrome/browser/ui/intents/web_intents_model.h:90: virtual ~Observer() {} On 2012/10/09 01:58:22, groby wrote: > ...

8 years, 2 months ago (2012-10-09 02:00:33 UTC) #3

still lgtm :)

Expand Messages | Collapse Messages