chrome/browser/search/one_google_bar/one_google_bar_service_observer.h - Issue 2811353002: Local NTP: Introduce OneGoogleBarService

Side by Side Diff: chrome/browser/search/one_google_bar/one_google_bar_service_observer.h

Issue 2811353002: Local NTP: Introduce OneGoogleBarService (Closed)

Patch Set: . Created 3 years, 8 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

« chrome/browser/search/one_google_bar/one_google_bar_service.cc ('K') | « chrome/browser/search/one_google_bar/one_google_bar_service_factory.cc ('k') | chrome/browser/search/one_google_bar/one_google_bar_service_unittest.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 // Copyright 2017 The Chromium Authors. All rights reserved.

	2 // Use of this source code is governed by a BSD-style license that can be

	3 // found in the LICENSE file.

	4

	5 #ifndef CHROME_BROWSER_SEARCH_ONE_GOOGLE_BAR_ONE_GOOGLE_BAR_SERVICE_OBSERVER_H_

	6 #define CHROME_BROWSER_SEARCH_ONE_GOOGLE_BAR_ONE_GOOGLE_BAR_SERVICE_OBSERVER_H_

	7

	8 #include "base/optional.h"

	9

	10 struct OneGoogleBarData;

	11

	12 // Observer for OneGoogleBarService. Gets notified when the OneGoogleBarData

	13 // changes.

	14 class OneGoogleBarServiceObserver {

	15 public:

	16 virtual void OnOneGoogleBarDataChanged(

	17 const base::Optional<OneGoogleBarData>& data) = 0;
	sfiera 2017/04/13 15:18:31 Since the service provides an accessor, I would be Since the service provides an accessor, I would be inclined to provide either the service or nothing to the observer here, so that it's clear that there is one correct way to get the value. Marc Treib 2017/04/13 15:44:02 Hm. I do like the idea of "one way to get the valu Show quoted text On 2017/04/13 15:18:31, sfiera wrote: > Since the service provides an accessor, I would be inclined to provide either > the service or nothing to the observer here, so that it's clear that there is > one correct way to get the value. Hm. I do like the idea of "one way to get the value", but it's also somewhat less convenient for clients... sfiera 2017/04/13 15:58:33 Passing nothing would be a lot less convenient, si Show quoted text On 2017/04/13 15:44:02, Marc Treib (OOO till April 25) wrote: > On 2017/04/13 15:18:31, sfiera wrote: > > Since the service provides an accessor, I would be inclined to provide either > > the service or nothing to the observer here, so that it's clear that there is > > one correct way to get the value. > > Hm. I do like the idea of "one way to get the value", but it's also somewhat > less convenient for clients... Passing nothing would be a lot less convenient, since you'd have to save a pointer to the service. Passing the service seems OK to me; use service->one_google_bar_data() instead of data. Marc Treib 2017/04/13 16:45:15 Eeh, in my experience most observers need to keep Show quoted text On 2017/04/13 15:58:33, sfiera wrote: > On 2017/04/13 15:44:02, Marc Treib (OOO till April 25) wrote: > > On 2017/04/13 15:18:31, sfiera wrote: > > > Since the service provides an accessor, I would be inclined to provide > either > > > the service or nothing to the observer here, so that it's clear that there > is > > > one correct way to get the value. > > > > Hm. I do like the idea of "one way to get the value", but it's also somewhat > > less convenient for clients... > > Passing nothing would be a lot less convenient, since you'd have to save a > pointer to the service. Passing the service seems OK to me; use > service->one_google_bar_data() instead of data. Eeh, in my experience most observers need to keep a pointer to the service around anyway. Here in particular, the one pending implementation does that. I've removed the data param.
	18 };

	19

	20 #endif // CHROME_BROWSER_SEARCH_ONE_GOOGLE_BAR_ONE_GOOGLE_BAR_SERVICE_OBSERVER_ H_

OLD	NEW