Use invalid value for GetAccessPointName for CHROME_IOS_RESERVED

Use invalid value for GetAccessPointName for CHROME_IOS_RESERVED

The team that uses the RLZ access point does not want to reserve two
access points for Chrome on iOS since only one is currently used but
the enumeration rlz_lib::AccessPoint cannot be changed.

Mark the CHROME_IOS_RESERVED access point enumeration value as not
supported in IsAccessPointSupported (as we should never reference an
access point with enumeration value until the value is reused). This
fixes the code that iterates over all values in that enumeration in
FinancialPing::FormRequest when no events have been recorded.

Return an invalid value in GetAccessPointName if called with the
enumeration value rlz_lib::AccessPoint instead of returning NULL to
avoid a crash on Chrome OS (constructing a std::string from NULL crash
in strlen on Chrome OS).

BUG=360686, 363700
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=264574

[sdefresne, 2014-04-15 21:08:09]

Roger, please take a look at the code. I think that an access point of __ is an
invalid value and will be ignored by the processing pipeline, and thus should be
safe to use.

Haixia, can you check that this works on Chrome OS? I don't have an easy access
to that architecture to test, and the try bots don't build with RLZ enabled. I
think I fixed the crash that you encountered with the change in
IsAccessPointSupported (as this will cause the value to be skipped in
GetAccessPointRlz and thus rlz_lib::RlzValueStoreChromeOS::ReadAccessPointRlz
should never be called with that enumeration value).

[hshi1, 2014-04-15 21:12:32]

On 2014/04/15 21:08:09, sdefresne wrote:
> Roger, please take a look at the code. I think that an access point of __ is
an
> invalid value and will be ignored by the processing pipeline, and thus should
be
> safe to use.
> 
> Haixia, can you check that this works on Chrome OS? I don't have an easy
access
> to that architecture to test, and the try bots don't build with RLZ enabled. I
> think I fixed the crash that you encountered with the change in
> IsAccessPointSupported (as this will cause the value to be skipped in
> GetAccessPointRlz and thus rlz_lib::RlzValueStoreChromeOS::ReadAccessPointRlz
> should never be called with that enumeration value).

Hi Sylvain, I patched this CL locally on trunk Chrome OS and verified that it
works. No crashes occured after 2 minutes. Thanks!

[sdefresne, 2014-04-15 21:15:59]

On 2014/04/15 21:12:32, hshi1 wrote:
> On 2014/04/15 21:08:09, sdefresne wrote:
> > Roger, please take a look at the code. I think that an access point of __ is
> an
> > invalid value and will be ignored by the processing pipeline, and thus
should
> be
> > safe to use.
> > 
> > Haixia, can you check that this works on Chrome OS? I don't have an easy
> access
> > to that architecture to test, and the try bots don't build with RLZ enabled.
I
> > think I fixed the crash that you encountered with the change in
> > IsAccessPointSupported (as this will cause the value to be skipped in
> > GetAccessPointRlz and thus
rlz_lib::RlzValueStoreChromeOS::ReadAccessPointRlz
> > should never be called with that enumeration value).
> 
> Hi Sylvain, I patched this CL locally on trunk Chrome OS and verified that it
> works. No crashes occured after 2 minutes. Thanks!

Thank you for checking. I just wanted to know if you ensured that enabled_rlz
was enabled. AFAIK, it default to 0 on most architecture unless this is an
official build. I don't know if it is set to 1 by default on Chrome OS or not.
It may be a good idea to check after running "gclient runhooks" with GYP_DEFINES
environment variable set to enable_rlz=1.

[hshi1, 2014-04-15 21:26:11]

On 2014/04/15 21:15:59, sdefresne wrote:
> On 2014/04/15 21:12:32, hshi1 wrote:
> > On 2014/04/15 21:08:09, sdefresne wrote:
> > > Roger, please take a look at the code. I think that an access point of __
is
> > an
> > > invalid value and will be ignored by the processing pipeline, and thus
> should
> > be
> > > safe to use.
> > > 
> > > Haixia, can you check that this works on Chrome OS? I don't have an easy
> > access
> > > to that architecture to test, and the try bots don't build with RLZ
enabled.
> I
> > > think I fixed the crash that you encountered with the change in
> > > IsAccessPointSupported (as this will cause the value to be skipped in
> > > GetAccessPointRlz and thus
> rlz_lib::RlzValueStoreChromeOS::ReadAccessPointRlz
> > > should never be called with that enumeration value).
> > 
> > Hi Sylvain, I patched this CL locally on trunk Chrome OS and verified that
it
> > works. No crashes occured after 2 minutes. Thanks!
> 
> Thank you for checking. I just wanted to know if you ensured that enabled_rlz
> was enabled. AFAIK, it default to 0 on most architecture unless this is an
> official build. I don't know if it is set to 1 by default on Chrome OS or not.
> It may be a good idea to check after running "gclient runhooks" with
GYP_DEFINES
> environment variable set to enable_rlz=1.

Yes it is an official build (the same build was previously crashing when
GetAccessPointName returned NULL pointer).

[sdefresne, 2014-04-15 21:27:25]

On 2014/04/15 21:26:11, hshi1 wrote:
> On 2014/04/15 21:15:59, sdefresne wrote:
> > On 2014/04/15 21:12:32, hshi1 wrote:
> > > On 2014/04/15 21:08:09, sdefresne wrote:
> > > > Roger, please take a look at the code. I think that an access point of
__
> is
> > > an
> > > > invalid value and will be ignored by the processing pipeline, and thus
> > should
> > > be
> > > > safe to use.
> > > > 
> > > > Haixia, can you check that this works on Chrome OS? I don't have an easy
> > > access
> > > > to that architecture to test, and the try bots don't build with RLZ
> enabled.
> > I
> > > > think I fixed the crash that you encountered with the change in
> > > > IsAccessPointSupported (as this will cause the value to be skipped in
> > > > GetAccessPointRlz and thus
> > rlz_lib::RlzValueStoreChromeOS::ReadAccessPointRlz
> > > > should never be called with that enumeration value).
> > > 
> > > Hi Sylvain, I patched this CL locally on trunk Chrome OS and verified that
> it
> > > works. No crashes occured after 2 minutes. Thanks!
> > 
> > Thank you for checking. I just wanted to know if you ensured that
enabled_rlz
> > was enabled. AFAIK, it default to 0 on most architecture unless this is an
> > official build. I don't know if it is set to 1 by default on Chrome OS or
not.
> > It may be a good idea to check after running "gclient runhooks" with
> GYP_DEFINES
> > environment variable set to enable_rlz=1.
> 
> Yes it is an official build (the same build was previously crashing when
> GetAccessPointName returned NULL pointer).

OK, thank you for confirming that the fix does work, and sorry for the trouble.

[hshi1, 2014-04-16 01:08:43]

lgtm

[Roger Tawa OOO till Jul 10th, 2014-04-17 15:08:48]

lgtm

[sdefresne, 2014-04-17 16:13:15]

The CQ bit was checked by sdefresne@chromium.org

[commit-bot: I haz the power, 2014-04-17 16:13:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sdefresne@chromium.org/239053004/20001

[commit-bot: I haz the power, 2014-04-17 16:46:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-17 16:46:49]

Retried try job too often on ios_rel_device for step(s) compile
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_rel_de...

[sdefresne, 2014-04-17 17:25:23]

The CQ bit was checked by sdefresne@chromium.org

[commit-bot: I haz the power, 2014-04-17 17:26:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sdefresne@chromium.org/239053004/20001

[commit-bot: I haz the power, 2014-04-17 18:10:37]

Change committed as 264574