chrome/browser should build on the x86_64 architecture

chrome/browser should build on the x86_64 architecture.

BUG=143395
TEST=32/64-bit chrome/browser builds without errors.
TBR=mac64

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

[Cătălin Badea, 2012-08-20 12:42:41]

Please review this change that enables chrome/browser to build on the x86_64
architecture.

[Mark Mentovai, 2012-08-20 13:00:13]

We need to put this on hold until we’ve gotten the legal stuff out of the way.
We’ve got to make sure Adobe’s authorized you as a contributor to Google
open-source projects.

[Cătălin Badea, 2012-08-20 13:10:22]

On 2012/08/20 13:00:13, Mark Mentovai wrote:
> We need to put this on hold until we’ve gotten the legal stuff out of the way.
> We’ve got to make sure Adobe’s authorized you as a contributor to Google
> open-source projects.

Yes, I'm in the process of being added on the authorized contributors list.

[Mark Mentovai, 2012-08-20 13:25:58]

Here is some early feedback, then.

I think you’ve probably overdone it on the reviewer selection. For Mac 64-bit
changes, you can send your patches to me, and worry about placating the commit
queue by getting OWNERS only once you’re truly ready to commit.

https://chromiumcodereview.appspot.com/10825447/diff/1/chrome/browser/chrome_...
File chrome/browser/chrome_browser_application_mac.mm (right):

https://chromiumcodereview.appspot.com/10825447/diff/1/chrome/browser/chrome_...
chrome/browser/chrome_browser_application_mac.mm:393: static_cast<int32_t>(tag),
The correct way to deal with NSInteger is a cast to long (and a format switch to
%ld). Similarly, NSUInteger is dealt with by a cast to unsigned long and %lu.
See
https://developer.apple.com/library/mac/#documentation/Cocoa/Conceptual/Strin...
.

https://chromiumcodereview.appspot.com/10825447/diff/1/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/about_ipc_controller.mm (right):

https://chromiumcodereview.appspot.com/10825447/diff/1/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/about_ipc_controller.mm:188: [eventCount_
setStringValue:[NSString stringWithFormat:@"%d",
Same.

https://chromiumcodereview.appspot.com/10825447/diff/1/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm (right):

https://chromiumcodereview.appspot.com/10825447/diff/1/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm:562: CGFloat
alpha = std::max(static_cast<CGFloat>(0.0f),
“0.0” means “zero as a double,” whereas “0.0f” means “zero as a float,” or
“static_cast<float>(0.0).” CGFloat will now be either a float or a double. When
it’s a double and you write “static_cast<CGFloat>(0.0f),” you’re effectively
saying “take 0.0, cast it to a float, and then cast it to a double.”

You should change these from 0.0f to 0.0.

This is in line with Chrome’s other uses of CGFloat literal values which just
use doubles (no f), such as the ones a few lines above.

https://chromiumcodereview.appspot.com/10825447/diff/1/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/history_overlay_controller.mm (right):

https://chromiumcodereview.appspot.com/10825447/diff/1/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/history_overlay_controller.mm:115: CGFloat shieldAlpha =
std::min(static_cast<CGFloat>(0.65f),
Again.

https://chromiumcodereview.appspot.com/10825447/diff/1/third_party/mozilla/NS...
File third_party/mozilla/NSWorkspace+Utils.m (right):

https://chromiumcodereview.appspot.com/10825447/diff/1/third_party/mozilla/NS...
third_party/mozilla/NSWorkspace+Utils.m:167: static SInt32 sSystemVersion = 0;
When you make a change to most files in a third_party directory, you also need
to update the README.chromium file in its directory to note what change you’ve
made, and sometimes update a patch file containing that change. In this case,
there is a README.chromium but no patch file to worry about.

[sky, 2012-08-20 14:11:25]

You've got enough other reviewers. I've removed myself.

[Cătălin Badea, 2012-08-20 15:12:23]

Thanks for the feedback. I have uploaded a new patch set.

[Mark Mentovai, 2012-08-21 13:44:54]

LGTM

We’ve also gotten an updated list of authorized contributors from Adobe and
you’re on it.

[commit-bot: I haz the power, 2012-08-21 13:45:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/badea@adobe.com/10825447/1014

[commit-bot: I haz the power, 2012-08-21 13:45:27]

Failed to apply patch for AUTHORS:
While running patch -p1 --forward --force;
patching file AUTHORS
Hunk #1 FAILED at 197.
1 out of 1 hunk FAILED -- saving rejects to file AUTHORS.rej

[Mark Mentovai, 2012-08-21 13:46:17]

Can you upload a current version that applies cleanly to the current trunk
AUTHORS file?

[Cătălin Badea, 2012-08-21 13:58:49]

Patch should apply now.

[Mark Mentovai, 2012-08-21 14:07:46]

LGTM and CQ

[commit-bot: I haz the power, 2012-08-21 14:08:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/badea@adobe.com/10825447/2004

[commit-bot: I haz the power, 2012-08-21 16:33:57]

Change committed as 152572