Initial iOS bring up of ui.

Initial iOS bring up of ui.

A very bare minimum: only point, rect and size are compiled and no unittest support yet.

BUG=None


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

[noyau (Ping after 24h), 2012-08-01 10:43:23]

To Stuart for initial review.

I made two uploads. I prefer the second version, removing the ifdef for mac and
iOS from the .cc. The necessary definitions are already present in the included
headers as they are structs and can't be forward declared.

The first upload stays true to the original code.

Let me know if you prefer the first version, I'll revert those changes.

[stuartmorgan, 2012-08-01 13:26:18]

http://codereview.chromium.org/10837041/diff/2001/ui/gfx/point.h
File ui/gfx/point.h (right):

http://codereview.chromium.org/10837041/diff/2001/ui/gfx/point.h#newcode16
ui/gfx/point.h:16: #elif defined(OS_MACOSX)  // iOS special-cased above
Let's remove these comments; I'm not sure they add anything.

http://codereview.chromium.org/10837041/diff/2001/ui/gfx/rect.cc
File ui/gfx/rect.cc (left):

http://codereview.chromium.org/10837041/diff/2001/ui/gfx/rect.cc#oldcode10
ui/gfx/rect.cc:10: #include <ApplicationServices/ApplicationServices.h>
I think this is fine to remove; if we really want to have this header for
CGRect, then the right place for it would be rect.h (and we'd want to do the
same thing we do in point.h). Really we *should* have it in the header, for
IWYU, so let's do that; right now we are relying on CGPoint needing it, even
though our use here is for CGRect, which is slightly shady.

http://codereview.chromium.org/10837041/diff/2001/ui/gfx/size.cc
File ui/gfx/size.cc (left):

http://codereview.chromium.org/10837041/diff/2001/ui/gfx/size.cc#oldcode10
ui/gfx/size.cc:10: #include <ApplicationServices/ApplicationServices.h>
This is clearly the right thing to do since it's in this file's header already.

http://codereview.chromium.org/10837041/diff/2001/ui/gfx/size.h
File ui/gfx/size.h (right):

http://codereview.chromium.org/10837041/diff/2001/ui/gfx/size.h#newcode18
ui/gfx/size.h:18: #elif defined(OS_MACOSX)  // iOS special-cased above
Remove comments

http://codereview.chromium.org/10837041/diff/2001/ui/ui.gyp
File ui/ui.gyp (left):

http://codereview.chromium.org/10837041/diff/2001/ui/ui.gyp#oldcode23
ui/ui.gyp:23: '../base/base.gyp:base_static',
Why do we have to remove these?

http://codereview.chromium.org/10837041/diff/2001/ui/ui.gyp#oldcode31
ui/ui.gyp:31: '../third_party/zlib/zlib.gyp:zlib',
And this?

http://codereview.chromium.org/10837041/diff/2001/ui/ui.gyp#oldcode33
ui/ui.gyp:33: 'ui_resources',
Can't we use dependencies! for some of these?

http://codereview.chromium.org/10837041/diff/2001/ui/ui.gyp
File ui/ui.gyp (right):

http://codereview.chromium.org/10837041/diff/2001/ui/ui.gyp#newcode464
ui/ui.gyp:464: 'dependencies': [
Indentation is wrong here.

http://codereview.chromium.org/10837041/diff/2001/ui/ui.gyp#newcode474
ui/ui.gyp:474: ],
And this should be on the previous line, with the comma from the previous line
removed.

http://codereview.chromium.org/10837041/diff/2001/ui/ui.gyp#newcode686
ui/ui.gyp:686: ['OS == "ios"', {
Everything else in this block has the no-spaces style, so let's match that.

http://codereview.chromium.org/10837041/diff/2001/ui/ui.gyp#newcode689
ui/ui.gyp:689: # everything and then just select the parts needed.
Will this always be true? If not, change to a TODO; if so, add a TODO after it
to add files here as they are made iOS-ready, so it's clear this isn't a final
list.

http://codereview.chromium.org/10837041/diff/2001/ui/ui.gyp#newcode705
ui/ui.gyp:705: '$(SDKROOT)/System/Library/Frameworks/CoreGraphics.framework',
Indent more.

http://codereview.chromium.org/10837041/diff/2001/ui/ui.gyp#newcode709
ui/ui.gyp:709: ['use_x11 == 1', {
Don't change the spacing here.

http://codereview.chromium.org/10837041/diff/2001/ui/ui.gyp#newcode770
ui/ui.gyp:770: }],
This block shouldn't be necessary upstream; nothing runs unit tests yet.

http://codereview.chromium.org/10837041/diff/2001/ui/ui.gyp#newcode772
ui/ui.gyp:772: # TODO(ios): The ui tests do not compile yet on iOS. See
b/4935009.
That bug has nothing to do with compiling AFAICT. Just remove the bug reference;
this early in the ui/ upstreaming the TODO speaks for itself.

[noyau (Ping after 24h), 2012-08-01 13:53:45]

PTAL.

https://chromiumcodereview.appspot.com/10837041/diff/2001/ui/gfx/point.h
File ui/gfx/point.h (right):

https://chromiumcodereview.appspot.com/10837041/diff/2001/ui/gfx/point.h#newc...
ui/gfx/point.h:16: #elif defined(OS_MACOSX)  // iOS special-cased above
On 2012/08/01 13:26:18, stuartmorgan wrote:
> Let's remove these comments; I'm not sure they add anything.

Done.

https://chromiumcodereview.appspot.com/10837041/diff/2001/ui/gfx/rect.cc
File ui/gfx/rect.cc (left):

https://chromiumcodereview.appspot.com/10837041/diff/2001/ui/gfx/rect.cc#oldc...
ui/gfx/rect.cc:10: #include <ApplicationServices/ApplicationServices.h>
On 2012/08/01 13:26:18, stuartmorgan wrote:
> I think this is fine to remove; if we really want to have this header for
> CGRect, then the right place for it would be rect.h (and we'd want to do the
> same thing we do in point.h). Really we *should* have it in the header, for
> IWYU, so let's do that; right now we are relying on CGPoint needing it, even
> though our use here is for CGRect, which is slightly shady.

Done.

https://chromiumcodereview.appspot.com/10837041/diff/2001/ui/gfx/size.cc
File ui/gfx/size.cc (left):

https://chromiumcodereview.appspot.com/10837041/diff/2001/ui/gfx/size.cc#oldc...
ui/gfx/size.cc:10: #include <ApplicationServices/ApplicationServices.h>
On 2012/08/01 13:26:18, stuartmorgan wrote:
> This is clearly the right thing to do since it's in this file's header
already.

Done.

https://chromiumcodereview.appspot.com/10837041/diff/2001/ui/gfx/size.h
File ui/gfx/size.h (right):

https://chromiumcodereview.appspot.com/10837041/diff/2001/ui/gfx/size.h#newco...
ui/gfx/size.h:18: #elif defined(OS_MACOSX)  // iOS special-cased above
On 2012/08/01 13:26:18, stuartmorgan wrote:
> Remove comments

Done.

https://chromiumcodereview.appspot.com/10837041/diff/2001/ui/ui.gyp
File ui/ui.gyp (left):

https://chromiumcodereview.appspot.com/10837041/diff/2001/ui/ui.gyp#oldcode33
ui/ui.gyp:33: 'ui_resources',
On 2012/08/01 13:26:18, stuartmorgan wrote:
> Can't we use dependencies! for some of these?
I restored them all except for libjpeg, which cannot be excluded with
dependencies! as it is not present.

https://chromiumcodereview.appspot.com/10837041/diff/2001/ui/ui.gyp
File ui/ui.gyp (right):

https://chromiumcodereview.appspot.com/10837041/diff/2001/ui/ui.gyp#newcode464
ui/ui.gyp:464: 'dependencies': [
On 2012/08/01 13:26:18, stuartmorgan wrote:
> Indentation is wrong here.

Done.

https://chromiumcodereview.appspot.com/10837041/diff/2001/ui/ui.gyp#newcode474
ui/ui.gyp:474: ],
On 2012/08/01 13:26:18, stuartmorgan wrote:
> And this should be on the previous line, with the comma from the previous line
> removed.

Done.

https://chromiumcodereview.appspot.com/10837041/diff/2001/ui/ui.gyp#newcode686
ui/ui.gyp:686: ['OS == "ios"', {
On 2012/08/01 13:26:18, stuartmorgan wrote:
> Everything else in this block has the no-spaces style, so let's match that.

Done.

https://chromiumcodereview.appspot.com/10837041/diff/2001/ui/ui.gyp#newcode689
ui/ui.gyp:689: # everything and then just select the parts needed.
On 2012/08/01 13:26:18, stuartmorgan wrote:
> Will this always be true? If not, change to a TODO; if so, add a TODO after it
> to add files here as they are made iOS-ready, so it's clear this isn't a final
> list.

Done.

https://chromiumcodereview.appspot.com/10837041/diff/2001/ui/ui.gyp#newcode705
ui/ui.gyp:705: '$(SDKROOT)/System/Library/Frameworks/CoreGraphics.framework',
On 2012/08/01 13:26:18, stuartmorgan wrote:
> Indent more.

Done.

https://chromiumcodereview.appspot.com/10837041/diff/2001/ui/ui.gyp#newcode709
ui/ui.gyp:709: ['use_x11 == 1', {
On 2012/08/01 13:26:18, stuartmorgan wrote:
> Don't change the spacing here.

Done.

https://chromiumcodereview.appspot.com/10837041/diff/2001/ui/ui.gyp#newcode770
ui/ui.gyp:770: }],
On 2012/08/01 13:26:18, stuartmorgan wrote:
> This block shouldn't be necessary upstream; nothing runs unit tests yet.

Done.

https://chromiumcodereview.appspot.com/10837041/diff/2001/ui/ui.gyp#newcode772
ui/ui.gyp:772: # TODO(ios): The ui tests do not compile yet on iOS. See
b/4935009.
On 2012/08/01 13:26:18, stuartmorgan wrote:
> That bug has nothing to do with compiling AFAICT. Just remove the bug
reference;
> this early in the ui/ upstreaming the TODO speaks for itself.

Done.

[stuartmorgan, 2012-08-01 14:48:52]

LGTM

[noyau (Ping after 24h), 2012-08-01 15:06:58]

To sky@ for owner review.

[sky, 2012-08-01 15:46:07]

LGTM

[commit-bot: I haz the power, 2012-08-01 15:59:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noyau@chromium.org/10837041/12001

[commit-bot: I haz the power, 2012-08-01 17:27:33]

Change committed as 149435