Use non-static set_key interface on DataReductionProxySettings

Use non-static set_key interface on DataReductionProxySettings

This removes the use of a static initializer and makes it possible
for clients to pass a key without owning the memory where the
key is stored (as would be the case if passed to a static const char*).

BUG=371626, 371204
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=269578

[bengr, 2014-05-09 00:11:44]

mmenke: profile_impl_io_data.cc
sgurun: android_webview
marq: everything else

[bengr, 2014-05-09 00:18:30]

This also fixes 371204.

[sgurun-gerrit only, 2014-05-09 00:58:28]

https://codereview.chromium.org/279633003/diff/40001/android_webview/browser/...
File android_webview/browser/net/aw_url_request_context_getter.cc (right):

https://codereview.chromium.org/279633003/diff/40001/android_webview/browser/...
android_webview/browser/net/aw_url_request_context_getter.cc:225:
main_cache->GetSession(), drp_settings->key());
I am not sure about this logic. The key may not be set by the time
InitDataReductionProxySession is called. Does this have to happen here?

https://codereview.chromium.org/279633003/diff/40001/android_webview/native/a...
File android_webview/native/aw_contents_statics.cc (right):

https://codereview.chromium.org/279633003/diff/40001/android_webview/native/a...
android_webview/native/aw_contents_statics.cc:58:
drp_settings->set_key(ConvertJavaStringToUTF8(env, key));
nit: alignment

[bengr, 2014-05-09 03:16:51]

https://codereview.chromium.org/279633003/diff/40001/android_webview/browser/...
File android_webview/browser/net/aw_url_request_context_getter.cc (right):

https://codereview.chromium.org/279633003/diff/40001/android_webview/browser/...
android_webview/browser/net/aw_url_request_context_getter.cc:225:
main_cache->GetSession(), drp_settings->key());
On 2014/05/09 00:58:28, sgurun wrote:
> I am not sure about this logic. The key may not be set by the time
> InitDataReductionProxySession is called. Does this have to happen here?

InitDataReductionProxySession constructs credentials to be used. It must be
called zero or one time, and if it is called, it must be called before any
requests are sent to the data reduction proxy. Do you have a better place? 

For now, we could remove this call, and credentials would be constructed on the
first request. (The data reduction proxy responds with an authentication
challenge if credentials are not provided.) The drawback to removing this call
is the latency of an extra round trip on the first request of each session.

[mmenke, 2014-05-09 15:27:08]

https://codereview.chromium.org/279633003/diff/50001/chrome/browser/profiles/...
File chrome/browser/profiles/profile_impl_io_data.cc (right):

https://codereview.chromium.org/279633003/diff/50001/chrome/browser/profiles/...
chrome/browser/profiles/profile_impl_io_data.cc:465: #if
defined(SPDY_PROXY_AUTH_VALUE)
Is this double #if really needed?

https://codereview.chromium.org/279633003/diff/50001/components/data_reductio...
File components/data_reduction_proxy/browser/data_reduction_proxy_settings.cc
(right):

https://codereview.chromium.org/279633003/diff/50001/components/data_reductio...
components/data_reduction_proxy/browser/data_reduction_proxy_settings.cc:105: :
key_(),
nit:  Initializer not needed.

https://codereview.chromium.org/279633003/diff/50001/components/data_reductio...
components/data_reduction_proxy/browser/data_reduction_proxy_settings.cc:127: 
nit:  Remove extra blank line.

https://codereview.chromium.org/279633003/diff/50001/components/data_reductio...
File components/data_reduction_proxy/browser/data_reduction_proxy_settings.h
(right):

https://codereview.chromium.org/279633003/diff/50001/components/data_reductio...
components/data_reduction_proxy/browser/data_reduction_proxy_settings.h:112:
std::string key();
nit:  When using this naming style, bodies should be inlined.

https://codereview.chromium.org/279633003/diff/50001/components/data_reductio...
components/data_reduction_proxy/browser/data_reduction_proxy_settings.h:112:
std::string key();
nit:  const std::string& key() const;

[bengr, 2014-05-09 15:47:27]

https://codereview.chromium.org/279633003/diff/50001/chrome/browser/profiles/...
File chrome/browser/profiles/profile_impl_io_data.cc (right):

https://codereview.chromium.org/279633003/diff/50001/chrome/browser/profiles/...
chrome/browser/profiles/profile_impl_io_data.cc:465: #if
defined(SPDY_PROXY_AUTH_VALUE)
On 2014/05/09 15:27:08, mmenke wrote:
> Is this double #if really needed?

Done.

https://codereview.chromium.org/279633003/diff/50001/components/data_reductio...
File components/data_reduction_proxy/browser/data_reduction_proxy_settings.cc
(right):

https://codereview.chromium.org/279633003/diff/50001/components/data_reductio...
components/data_reduction_proxy/browser/data_reduction_proxy_settings.cc:105: :
key_(),
On 2014/05/09 15:27:08, mmenke wrote:
> nit:  Initializer not needed.

Done.

https://codereview.chromium.org/279633003/diff/50001/components/data_reductio...
components/data_reduction_proxy/browser/data_reduction_proxy_settings.cc:127: 
On 2014/05/09 15:27:08, mmenke wrote:
> nit:  Remove extra blank line.

I inlined.

https://codereview.chromium.org/279633003/diff/50001/components/data_reductio...
File components/data_reduction_proxy/browser/data_reduction_proxy_settings.h
(right):

https://codereview.chromium.org/279633003/diff/50001/components/data_reductio...
components/data_reduction_proxy/browser/data_reduction_proxy_settings.h:112:
std::string key();
On 2014/05/09 15:27:08, mmenke wrote:
> nit:  When using this naming style, bodies should be inlined.

Done.

https://codereview.chromium.org/279633003/diff/50001/components/data_reductio...
components/data_reduction_proxy/browser/data_reduction_proxy_settings.h:112:
std::string key();
On 2014/05/09 15:27:08, mmenke wrote:
> nit:  When using this naming style, bodies should be inlined.

Done.

[mmenke, 2014-05-09 15:52:03]

profiles/ LGTM.

[sgurun-gerrit only, 2014-05-09 16:39:25]

https://codereview.chromium.org/279633003/diff/130001/components/data_reducti...
File components/data_reduction_proxy/browser/data_reduction_proxy_settings.cc
(right):

https://codereview.chromium.org/279633003/diff/130001/components/data_reducti...
components/data_reduction_proxy/browser/data_reduction_proxy_settings.cc:173: //
This is a no-op unless the authentication parameters are compiled in.
Is this comment stale? which parameters are compiled in now.

[marq (ping after 24h), 2014-05-09 17:00:05]

LGTM with formatting nits. Appears no have no downstream impact on iOS.

https://codereview.chromium.org/279633003/diff/150001/components/data_reducti...
File components/data_reduction_proxy/browser/data_reduction_proxy_settings.h
(right):

https://codereview.chromium.org/279633003/diff/150001/components/data_reducti...
components/data_reduction_proxy/browser/data_reduction_proxy_settings.h:114:
const std::string& key() const {
newline between methods.

https://codereview.chromium.org/279633003/diff/150001/components/data_reducti...
components/data_reduction_proxy/browser/data_reduction_proxy_settings.h:179:
base::string16 GetTokenForAuthChallenge(
Fits on one line now.

https://codereview.chromium.org/279633003/diff/150001/components/data_reducti...
components/data_reduction_proxy/browser/data_reduction_proxy_settings.h:316:
const std::string& key);
Fits on one line?

[bengr, 2014-05-09 17:12:20]

https://codereview.chromium.org/279633003/diff/130001/components/data_reducti...
File components/data_reduction_proxy/browser/data_reduction_proxy_settings.cc
(right):

https://codereview.chromium.org/279633003/diff/130001/components/data_reducti...
components/data_reduction_proxy/browser/data_reduction_proxy_settings.cc:173: //
This is a no-op unless the authentication parameters are compiled in.
On 2014/05/09 16:39:25, sgurun wrote:
> Is this comment stale? which parameters are compiled in now.

Done.

https://codereview.chromium.org/279633003/diff/150001/components/data_reducti...
File components/data_reduction_proxy/browser/data_reduction_proxy_settings.h
(right):

https://codereview.chromium.org/279633003/diff/150001/components/data_reducti...
components/data_reduction_proxy/browser/data_reduction_proxy_settings.h:114:
const std::string& key() const {
On 2014/05/09 17:00:06, marq_ wrote:
> newline between methods.

Done.

https://codereview.chromium.org/279633003/diff/150001/components/data_reducti...
components/data_reduction_proxy/browser/data_reduction_proxy_settings.h:179:
base::string16 GetTokenForAuthChallenge(
On 2014/05/09 17:00:06, marq_ wrote:
> Fits on one line now.

Done.

https://codereview.chromium.org/279633003/diff/150001/components/data_reducti...
components/data_reduction_proxy/browser/data_reduction_proxy_settings.h:316:
const std::string& key);
On 2014/05/09 17:00:06, marq_ wrote:
> Fits on one line?

Done.

[sgurun-gerrit only, 2014-05-09 17:40:26]

https://codereview.chromium.org/279633003/diff/190001/android_webview/native/...
File android_webview/native/aw_contents_statics.cc (right):

https://codereview.chromium.org/279633003/diff/190001/android_webview/native/...
android_webview/native/aw_contents_statics.cc:58:
drp_settings->set_key(ConvertJavaStringToUTF8(env, key));
fix the alignment?

[bengr, 2014-05-09 17:54:23]

https://codereview.chromium.org/279633003/diff/190001/android_webview/native/...
File android_webview/native/aw_contents_statics.cc (right):

https://codereview.chromium.org/279633003/diff/190001/android_webview/native/...
android_webview/native/aw_contents_statics.cc:58:
drp_settings->set_key(ConvertJavaStringToUTF8(env, key));
On 2014/05/09 17:40:26, sgurun wrote:
> fix the alignment?

Done.

[sgurun-gerrit only, 2014-05-09 23:50:09]

On 2014/05/09 17:54:23, bengr1 wrote:
>
https://codereview.chromium.org/279633003/diff/190001/android_webview/native/...
> File android_webview/native/aw_contents_statics.cc (right):
> 
>
https://codereview.chromium.org/279633003/diff/190001/android_webview/native/...
> android_webview/native/aw_contents_statics.cc:58:
> drp_settings->set_key(ConvertJavaStringToUTF8(env, key));
> On 2014/05/09 17:40:26, sgurun wrote:
> > fix the alignment?
> 
> Done.

aw lgtm 
tests need to be added later. the initialization order is fragile so need aw
tests for this.

[bengr, 2014-05-10 03:17:17]

The CQ bit was checked by bengr@chromium.org

[commit-bot: I haz the power, 2014-05-10 03:20:52]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bengr@chromium.org/279633003/320001

[bengr, 2014-05-10 04:44:19]

The CQ bit was checked by bengr@chromium.org

[commit-bot: I haz the power, 2014-05-10 04:45:39]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bengr@chromium.org/279633003/310014

[commit-bot: I haz the power, 2014-05-10 09:02:26]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-10 12:16:53]

Change committed as 269578

[piman, 2014-05-12 19:19:23]

https://codereview.chromium.org/279633003/diff/50001/chrome/browser/profiles/...
File chrome/browser/profiles/profile_impl_io_data.cc (right):

https://codereview.chromium.org/279633003/diff/50001/chrome/browser/profiles/...
chrome/browser/profiles/profile_impl_io_data.cc:465: #if
defined(SPDY_PROXY_AUTH_VALUE)
On 2014/05/09 15:27:08, mmenke wrote:
> Is this double #if really needed?

Actually... it is.
clank/supplemental.gypi which makes this defined is sourced if present,
regardless of the platform. However, DataReductionProxySettings is not present
on regular linux builds, so if you build that with clank/ checked out, it fails
to build.

[mmenke, 2014-05-12 19:21:57]

https://codereview.chromium.org/279633003/diff/50001/chrome/browser/profiles/...
File chrome/browser/profiles/profile_impl_io_data.cc (right):

https://codereview.chromium.org/279633003/diff/50001/chrome/browser/profiles/...
chrome/browser/profiles/profile_impl_io_data.cc:465: #if
defined(SPDY_PROXY_AUTH_VALUE)
On 2014/05/12 19:19:24, piman wrote:
> On 2014/05/09 15:27:08, mmenke wrote:
> > Is this double #if really needed?
> 
> Actually... it is.
> clank/supplemental.gypi which makes this defined is sourced if present,
> regardless of the platform. However, DataReductionProxySettings is not present
> on regular linux builds, so if you build that with clank/ checked out, it
fails
> to build.

I think it would make more sense to make it not defined, rather than to modify C
files.  Otherwise, we have SPDY_PROXY_AUTH_VALUE defined, but aren't really
supporting it, which is odd, and affects a bunch of other files as well.

[piman, 2014-05-12 19:28:25]

On Mon, May 12, 2014 at 12:21 PM, <mmenke@chromium.org> wrote:

>
> https://codereview.chromium.org/279633003/diff/50001/
> chrome/browser/profiles/profile_impl_io_data.cc
> File chrome/browser/profiles/profile_impl_io_data.cc (right):
>
> https://codereview.chromium.org/279633003/diff/50001/
> chrome/browser/profiles/profile_impl_io_data.cc#newcode465
> chrome/browser/profiles/profile_impl_io_data.cc:465: #if
> defined(SPDY_PROXY_AUTH_VALUE)
> On 2014/05/12 19:19:24, piman wrote:
>
>> On 2014/05/09 15:27:08, mmenke wrote:
>> > Is this double #if really needed?
>>
>
>  Actually... it is.
>> clank/supplemental.gypi which makes this defined is sourced if
>>
> present,
>
>> regardless of the platform. However, DataReductionProxySettings is not
>>
> present
>
>> on regular linux builds, so if you build that with clank/ checked out,
>>
> it fails
>
>> to build.
>>
>
> I think it would make more sense to make it not defined, rather than to
> modify C files.  Otherwise, we have SPDY_PROXY_AUTH_VALUE defined, but
> aren't really supporting it, which is odd, and affects a bunch of other
> files as well.
>

Maybe... you mean by changing supplemental.gypi to define the variables in
a condition block? or changing common.gypi to not add the defines on the
platforms where it's not supported?
Either way, do you mind sending a patch?


> https://codereview.chromium.org/279633003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[mmenke, 2014-05-12 19:40:21]

On 2014/05/12 19:28:25, piman wrote:
> On Mon, May 12, 2014 at 12:21 PM, <mailto:mmenke@chromium.org> wrote:
> 
> >
> > https://codereview.chromium.org/279633003/diff/50001/
> > chrome/browser/profiles/profile_impl_io_data.cc
> > File chrome/browser/profiles/profile_impl_io_data.cc (right):
> >
> > https://codereview.chromium.org/279633003/diff/50001/
> > chrome/browser/profiles/profile_impl_io_data.cc#newcode465
> > chrome/browser/profiles/profile_impl_io_data.cc:465: #if
> > defined(SPDY_PROXY_AUTH_VALUE)
> > On 2014/05/12 19:19:24, piman wrote:
> >
> >> On 2014/05/09 15:27:08, mmenke wrote:
> >> > Is this double #if really needed?
> >>
> >
> >  Actually... it is.
> >> clank/supplemental.gypi which makes this defined is sourced if
> >>
> > present,
> >
> >> regardless of the platform. However, DataReductionProxySettings is not
> >>
> > present
> >
> >> on regular linux builds, so if you build that with clank/ checked out,
> >>
> > it fails
> >
> >> to build.
> >>
> >
> > I think it would make more sense to make it not defined, rather than to
> > modify C files.  Otherwise, we have SPDY_PROXY_AUTH_VALUE defined, but
> > aren't really supporting it, which is odd, and affects a bunch of other
> > files as well.
> >
> 
> Maybe... you mean by changing supplemental.gypi to define the variables in
> a condition block? or changing common.gypi to not add the defines on the
> platforms where it's not supported?
> Either way, do you mind sending a patch?

Either one would be fine, but I was thinking of not including gypi files
intended specifically for clank in combined linux+clank checkouts.  Doing so
seems really odd to me.  I defer to bengr for the patch - I'm not familiar with
the plumbing that added the value.

[piman, 2014-05-12 19:44:04]

On Mon, May 12, 2014 at 12:40 PM, <mmenke@chromium.org> wrote:

> On 2014/05/12 19:28:25, piman wrote:
>
>> On Mon, May 12, 2014 at 12:21 PM, <mailto:mmenke@chromium.org> wrote:
>>
>
>  >
>> > https://codereview.chromium.org/279633003/diff/50001/
>> > chrome/browser/profiles/profile_impl_io_data.cc
>> > File chrome/browser/profiles/profile_impl_io_data.cc (right):
>> >
>> > https://codereview.chromium.org/279633003/diff/50001/
>>
>> > chrome/browser/profiles/profile_impl_io_data.cc#newcode465
>> > chrome/browser/profiles/profile_impl_io_data.cc:465: #if
>> > defined(SPDY_PROXY_AUTH_VALUE)
>> > On 2014/05/12 19:19:24, piman wrote:
>> >
>> >> On 2014/05/09 15:27:08, mmenke wrote:
>> >> > Is this double #if really needed?
>> >>
>> >
>> >  Actually... it is.
>> >> clank/supplemental.gypi which makes this defined is sourced if
>> >>
>> > present,
>> >
>> >> regardless of the platform. However, DataReductionProxySettings is not
>> >>
>> > present
>> >
>> >> on regular linux builds, so if you build that with clank/ checked out,
>> >>
>> > it fails
>> >
>> >> to build.
>> >>
>> >
>> > I think it would make more sense to make it not defined, rather than to
>> > modify C files.  Otherwise, we have SPDY_PROXY_AUTH_VALUE defined, but
>> > aren't really supporting it, which is odd, and affects a bunch of other
>> > files as well.
>> >
>>
>
>  Maybe... you mean by changing supplemental.gypi to define the variables in
>> a condition block? or changing common.gypi to not add the defines on the
>> platforms where it's not supported?
>> Either way, do you mind sending a patch?
>>
>
> Either one would be fine, but I was thinking of not including gypi files
> intended specifically for clank in combined linux+clank checkouts.  Doing
> so
> seems really odd to me.  I defer to bengr for the patch - I'm not familiar
> with
> the plumbing that added the value.
>

supplemental.gypi in direct subdirs are always included no matter what.
This is done in gyp_chromium, independently of the platform (original use
case is src-internal). I don't think that's the mechanism you want to tweak.
Or, you can have a miniature supplemental.gypi in clank/, that would
include supplemental-real.gypi under the platform condition.


> https://codereview.chromium.org/279633003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[bengr, 2014-05-12 20:07:50]

On 2014/05/12 19:44:04, piman wrote:
> On Mon, May 12, 2014 at 12:40 PM, <mailto:mmenke@chromium.org> wrote:
> 
> > On 2014/05/12 19:28:25, piman wrote:
> >
> >> On Mon, May 12, 2014 at 12:21 PM, <mailto:mmenke@chromium.org> wrote:
> >>
> >
> >  >
> >> > https://codereview.chromium.org/279633003/diff/50001/
> >> > chrome/browser/profiles/profile_impl_io_data.cc
> >> > File chrome/browser/profiles/profile_impl_io_data.cc (right):
> >> >
> >> > https://codereview.chromium.org/279633003/diff/50001/
> >>
> >> > chrome/browser/profiles/profile_impl_io_data.cc#newcode465
> >> > chrome/browser/profiles/profile_impl_io_data.cc:465: #if
> >> > defined(SPDY_PROXY_AUTH_VALUE)
> >> > On 2014/05/12 19:19:24, piman wrote:
> >> >
> >> >> On 2014/05/09 15:27:08, mmenke wrote:
> >> >> > Is this double #if really needed?
> >> >>
> >> >
> >> >  Actually... it is.
> >> >> clank/supplemental.gypi which makes this defined is sourced if
> >> >>
> >> > present,
> >> >
> >> >> regardless of the platform. However, DataReductionProxySettings is not
> >> >>
> >> > present
> >> >
> >> >> on regular linux builds, so if you build that with clank/ checked out,
> >> >>
> >> > it fails
> >> >
> >> >> to build.
> >> >>
> >> >
> >> > I think it would make more sense to make it not defined, rather than to
> >> > modify C files.  Otherwise, we have SPDY_PROXY_AUTH_VALUE defined, but
> >> > aren't really supporting it, which is odd, and affects a bunch of other
> >> > files as well.
> >> >
> >>
> >
> >  Maybe... you mean by changing supplemental.gypi to define the variables in
> >> a condition block? or changing common.gypi to not add the defines on the
> >> platforms where it's not supported?
> >> Either way, do you mind sending a patch?
> >>
> >
> > Either one would be fine, but I was thinking of not including gypi files
> > intended specifically for clank in combined linux+clank checkouts.  Doing
> > so
> > seems really odd to me.  I defer to bengr for the patch - I'm not familiar
> > with
> > the plumbing that added the value.
> >
> 
> supplemental.gypi in direct subdirs are always included no matter what.
> This is done in gyp_chromium, independently of the platform (original use
> case is src-internal). I don't think that's the mechanism you want to tweak.
> Or, you can have a miniature supplemental.gypi in clank/, that would
> include supplemental-real.gypi under the platform condition.
> 
> 
> > https://codereview.chromium.org/279633003/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Ultimately, I'm fine with doing whatever you think is best, but for now, I think
the most obvious course of action is to add the platform ifdefs back in. Shall I
do that? We can revisit this later if we add support for platforms other than
Android and iOS. OK?

[piman, 2014-05-12 20:09:55]

On Mon, May 12, 2014 at 1:07 PM, <bengr@chromium.org> wrote:

> On 2014/05/12 19:44:04, piman wrote:
>
>  On Mon, May 12, 2014 at 12:40 PM, <mailto:mmenke@chromium.org> wrote:
>>
>
>  > On 2014/05/12 19:28:25, piman wrote:
>> >
>> >> On Mon, May 12, 2014 at 12:21 PM, <mailto:mmenke@chromium.org> wrote:
>> >>
>> >
>> >  >
>> >> > https://codereview.chromium.org/279633003/diff/50001/
>> >> > chrome/browser/profiles/profile_impl_io_data.cc
>> >> > File chrome/browser/profiles/profile_impl_io_data.cc (right):
>> >> >
>> >> > https://codereview.chromium.org/279633003/diff/50001/
>> >>
>> >> > chrome/browser/profiles/profile_impl_io_data.cc#newcode465
>> >> > chrome/browser/profiles/profile_impl_io_data.cc:465: #if
>> >> > defined(SPDY_PROXY_AUTH_VALUE)
>> >> > On 2014/05/12 19:19:24, piman wrote:
>> >> >
>> >> >> On 2014/05/09 15:27:08, mmenke wrote:
>> >> >> > Is this double #if really needed?
>> >> >>
>> >> >
>> >> >  Actually... it is.
>> >> >> clank/supplemental.gypi which makes this defined is sourced if
>> >> >>
>> >> > present,
>> >> >
>> >> >> regardless of the platform. However, DataReductionProxySettings is
>> not
>> >> >>
>> >> > present
>> >> >
>> >> >> on regular linux builds, so if you build that with clank/ checked
>> out,
>> >> >>
>> >> > it fails
>> >> >
>> >> >> to build.
>> >> >>
>> >> >
>> >> > I think it would make more sense to make it not defined, rather than
>> to
>> >> > modify C files.  Otherwise, we have SPDY_PROXY_AUTH_VALUE defined,
>> but
>> >> > aren't really supporting it, which is odd, and affects a bunch of
>> other
>> >> > files as well.
>> >> >
>> >>
>> >
>> >  Maybe... you mean by changing supplemental.gypi to define the
>> variables in
>> >> a condition block? or changing common.gypi to not add the defines on
>> the
>> >> platforms where it's not supported?
>> >> Either way, do you mind sending a patch?
>> >>
>> >
>> > Either one would be fine, but I was thinking of not including gypi files
>> > intended specifically for clank in combined linux+clank checkouts.
>>  Doing
>> > so
>> > seems really odd to me.  I defer to bengr for the patch - I'm not
>> familiar
>> > with
>> > the plumbing that added the value.
>> >
>>
>
>  supplemental.gypi in direct subdirs are always included no matter what.
>> This is done in gyp_chromium, independently of the platform (original use
>> case is src-internal). I don't think that's the mechanism you want to
>> tweak.
>> Or, you can have a miniature supplemental.gypi in clank/, that would
>> include supplemental-real.gypi under the platform condition.
>>
>
>
>  > https://codereview.chromium.org/279633003/
>> >
>>
>
>  To unsubscribe from this group and stop receiving emails from it, send an
>>
> email
>
>> to mailto:chromium-reviews+unsubscribe@chromium.org.
>>
>
> Ultimately, I'm fine with doing whatever you think is best, but for now, I
> think
> the most obvious course of action is to add the platform ifdefs back in.
> Shall I
> do that? We can revisit this later if we add support for platforms other
> than
> Android and iOS. OK?
>

SGTM

>
> https://codereview.chromium.org/279633003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.