The changes for compiling with Android NDK r8b

These changes were from downstream and made the chromium compiled with Android NDK r8b

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=149460

[Yaron, 2012-07-27 17:34:30]

https://chromiumcodereview.appspot.com/10821061/diff/1/chrome/browser/search_...
File chrome/browser/search_engines/template_url_service.cc (right):

https://chromiumcodereview.appspot.com/10821061/diff/1/chrome/browser/search_...
chrome/browser/search_engines/template_url_service.cc:798: #if
defined(ENABLE_PROTECTOR_SERVICE)
Alternatively, you could push this #if define deeper into the if statement
guarding only the calls to the actual protector service.

[gavinp, 2012-07-27 17:40:27]

Drive by LGTM.

https://chromiumcodereview.appspot.com/10821061/diff/1/chrome/browser/prerend...
File chrome/browser/prerender/prerender_manager.cc (right):

https://chromiumcodereview.appspot.com/10821061/diff/1/chrome/browser/prerend...
chrome/browser/prerender/prerender_manager.cc:217: return NULL;
Thanks for catching this. I should have done this in my change.

[michaelbai, 2012-07-27 20:38:35]

https://chromiumcodereview.appspot.com/10821061/diff/1/chrome/browser/search_...
File chrome/browser/search_engines/template_url_service.cc (right):

https://chromiumcodereview.appspot.com/10821061/diff/1/chrome/browser/search_...
chrome/browser/search_engines/template_url_service.cc:798: #if
defined(ENABLE_PROTECTOR_SERVICE)
That will disable a lot of code, I don't know there are unit tests could verify
the changes. I'd prefer to add ALLOW_UNUSED.

On 2012/07/27 17:34:31, Yaron wrote:
> Alternatively, you could push this #if define deeper into the if statement
> guarding only the calls to the actual protector service.

[Yaron, 2012-07-27 20:40:43]

https://chromiumcodereview.appspot.com/10821061/diff/1/chrome/browser/search_...
File chrome/browser/search_engines/template_url_service.cc (right):

https://chromiumcodereview.appspot.com/10821061/diff/1/chrome/browser/search_...
chrome/browser/search_engines/template_url_service.cc:798: #if
defined(ENABLE_PROTECTOR_SERVICE)
On 2012/07/27 20:38:36, michaelbai wrote:
> That will disable a lot of code, I don't know there are unit tests could
verify
> the changes. I'd prefer to add ALLOW_UNUSED.
> 
> On 2012/07/27 17:34:31, Yaron wrote:
> > Alternatively, you could push this #if define deeper into the if statement
> > guarding only the calls to the actual protector service.
> 

No. I'm saying to move it to L804 or so and if-def less code. My concern is that
allowing it to be unused can cause other people to accidentally also not use it
when. Either way, I defer to sky@

[michaelbai, 2012-07-27 20:46:26]

sky is OOO, added John

[jam, 2012-07-27 23:47:17]

I'm overloaded with reviews for content, so for chrome please prefer to send it
to others, unless that code is something that I touch often. Redirecting to
jhawkins

[James Hawkins, 2012-07-30 03:09:08]

http://codereview.chromium.org/10821061/diff/1/chrome/browser/search_engines/...
File chrome/browser/search_engines/template_url_service.cc (right):

http://codereview.chromium.org/10821061/diff/1/chrome/browser/search_engines/...
chrome/browser/search_engines/template_url_service.cc:702: bool ALLOW_UNUSED
is_default_search_hijacked = false;
Why are you using ALLOW_UNUSED here?

[michaelbai, 2012-07-30 16:59:20]

http://codereview.chromium.org/10821061/diff/1/chrome/browser/search_engines/...
File chrome/browser/search_engines/template_url_service.cc (right):

http://codereview.chromium.org/10821061/diff/1/chrome/browser/search_engines/...
chrome/browser/search_engines/template_url_service.cc:702: bool ALLOW_UNUSED
is_default_search_hijacked = false;
I compiled this file by the gcc 4.6, there was a 
-werror=unused-but-set-variable error.

On 2012/07/30 03:09:09, James Hawkins wrote:
> Why are you using ALLOW_UNUSED here?

[sky, 2012-08-01 00:22:45]

http://codereview.chromium.org/10821061/diff/1/chrome/browser/search_engines/...
File chrome/browser/search_engines/template_url_service.cc (right):

http://codereview.chromium.org/10821061/diff/1/chrome/browser/search_engines/...
chrome/browser/search_engines/template_url_service.cc:798: #if
defined(ENABLE_PROTECTOR_SERVICE)
On 2012/07/27 20:40:43, Yaron wrote:
> On 2012/07/27 20:38:36, michaelbai wrote:
> > That will disable a lot of code, I don't know there are unit tests could
> verify
> > the changes. I'd prefer to add ALLOW_UNUSED.
> > 
> > On 2012/07/27 17:34:31, Yaron wrote:
> > > Alternatively, you could push this #if define deeper into the if statement
> > > guarding only the calls to the actual protector service.
> > 
> 
> No. I'm saying to move it to L804 or so and if-def less code. My concern is
that
> allowing it to be unused can cause other people to accidentally also not use
it
> when. Either way, I defer to sky@

Agree with Yaron. Move code into ifdef as necessary.

[michaelbai, 2012-08-01 01:49:22]

PTAL

http://codereview.chromium.org/10821061/diff/1/chrome/browser/search_engines/...
File chrome/browser/search_engines/template_url_service.cc (right):

http://codereview.chromium.org/10821061/diff/1/chrome/browser/search_engines/...
chrome/browser/search_engines/template_url_service.cc:798: #if
defined(ENABLE_PROTECTOR_SERVICE)
On 2012/08/01 00:22:45, sky wrote:
> On 2012/07/27 20:40:43, Yaron wrote:
> > On 2012/07/27 20:38:36, michaelbai wrote:
> > > That will disable a lot of code, I don't know there are unit tests could
> > verify
> > > the changes. I'd prefer to add ALLOW_UNUSED.
> > > 
> > > On 2012/07/27 17:34:31, Yaron wrote:
> > > > Alternatively, you could push this #if define deeper into the if
statement
> > > > guarding only the calls to the actual protector service.
> > > 
> > 
> > No. I'm saying to move it to L804 or so and if-def less code. My concern is
> that
> > allowing it to be unused can cause other people to accidentally also not use
> it
> > when. Either way, I defer to sky@
> 
> Agree with Yaron. Move code into ifdef as necessary.

Done.

[sky, 2012-08-01 02:04:16]

LGTM

[Yaron, 2012-08-01 17:53:23]

lgtm

[commit-bot: I haz the power, 2012-08-01 17:54:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/michaelbai@chromium.org/10821061/4008

[commit-bot: I haz the power, 2012-08-01 19:24:14]

Change committed as 149460