Integrate the Windows 8 code into the Chromium tree.

Integrate the Windows 8 code into the Chromium tree.

BUG=127799
TEST=A Chromium build can run as the immersive browser on Windows 8.


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

[robertshield, 2012-08-23 14:11:23]

Please take a look. I placed the code in the directory structure we agreed upon
in an earlier thread.

Thanks!
Robert

[MAD, 2012-08-23 14:41:39]

Small drive by request...

http://codereview.chromium.org/10875008/diff/10002/chrome/browser/ui/metro_dr...
File chrome/browser/ui/metro_driver/OWNERS (right):

http://codereview.chromium.org/10875008/diff/10002/chrome/browser/ui/metro_dr...
chrome/browser/ui/metro_driver/OWNERS:4: robertshield@chromium.org
Can you add me too?

[robertshield, 2012-08-23 14:45:34]

http://codereview.chromium.org/10875008/diff/10002/chrome/browser/ui/metro_dr...
File chrome/browser/ui/metro_driver/OWNERS (right):

http://codereview.chromium.org/10875008/diff/10002/chrome/browser/ui/metro_dr...
chrome/browser/ui/metro_driver/OWNERS:4: robertshield@chromium.org
On 2012/08/23 14:41:39, MAD wrote:
> Can you add me too?

Sure thing, done. Code reviews ho! :-)

[cpu_(ooo_6.6-7.5), 2012-08-23 20:15:17]

lgtm

http://codereview.chromium.org/10875008/diff/53/chrome/browser/ui/metro_drive...
File chrome/browser/ui/metro_driver/DEPS (right):

http://codereview.chromium.org/10875008/diff/53/chrome/browser/ui/metro_drive...
chrome/browser/ui/metro_driver/DEPS:4: "+win8/delegate_execute",
can you file bug and assign to ananta?

[Ben Goodger (Google), 2012-08-27 15:42:44]

So:

- what all does chrome/browser/ui/metro_driver do?
- is it chrome exe specific? It appears to depend only on itself.

Code that depends only on itself is a good candidate for not living in
src/chrome. The more code we can keep out of src/chrome the better. The only
exception would be if this is intricately coupled to the notion of the chrome
browser app. But even if there are some linkages here, is it possible this could
stand alone and be picked up & configured by src/chrome?

[robertshield, 2012-08-27 19:13:49]

On 2012/08/27 15:42:44, Ben Goodger (Google) wrote:
> So:
> 
> - what all does chrome/browser/ui/metro_driver do?

It is the bootstrap code for a metro process. It contains the creation of the
metro corewindow and message loop implementation. It also serves as the
intermediary between Chrome and all metro apis, housing a bunch of
feature-specific code (printing, file picking, etc.) that needs to interact with
metro.

> - is it chrome exe specific?

Somewhat, yes. The coupling between metro_driver and the parts of
chrome.exe/chrome.dll that live under an "if (IsMetro)" check is fairly tight.

Also, the long term goal is to move much of the code currently in
metro_driver.dll into chrome.dll.

> It appears to depend only on itself.

Yes, there is no compile time metro_driver -> chrome dependency.

> 
> Code that depends only on itself is a good candidate for not living in
> src/chrome. The more code we can keep out of src/chrome the better. The only
> exception would be if this is intricately coupled to the notion of the chrome
> browser app. But even if there are some linkages here, is it possible this
could
> stand alone and be picked up & configured by src/chrome?

It's very possible for this to stand alone and live in say src/win8/metro_driver
instead. There's a case for it to live both under chrome and not, and I don't
feel strongly either way.

The directory structure here was from a mild consensus we got from an earlier
mail thread we had (titled "Namespace(s) for Metro code?"), but I have no
objection to moving metro_driver out from under /chrome if you'd prefer. What
say you?

[robertshield, 2012-08-29 02:26:00]

Note to reviewers: I added a patch that looks for the Win8 SDK patch and errors
out with a descriptive message if it cannot be found.

PTAL at metro_driver.gyp, delegate_execute.gyp, check_sdk_patch.py.

@ben: Any preferences regarding the directory location for metro_driver?

On 2012/08/27 19:13:49, robertshield wrote:
> On 2012/08/27 15:42:44, Ben Goodger (Google) wrote:
> > So:
> > 
> > - what all does chrome/browser/ui/metro_driver do?
> 
> It is the bootstrap code for a metro process. It contains the creation of the
> metro corewindow and message loop implementation. It also serves as the
> intermediary between Chrome and all metro apis, housing a bunch of
> feature-specific code (printing, file picking, etc.) that needs to interact
with
> metro.
> 
> > - is it chrome exe specific?
> 
> Somewhat, yes. The coupling between metro_driver and the parts of
> chrome.exe/chrome.dll that live under an "if (IsMetro)" check is fairly tight.
> 
> Also, the long term goal is to move much of the code currently in
> metro_driver.dll into chrome.dll.
> 
> > It appears to depend only on itself.
> 
> Yes, there is no compile time metro_driver -> chrome dependency.
> 
> > 
> > Code that depends only on itself is a good candidate for not living in
> > src/chrome. The more code we can keep out of src/chrome the better. The only
> > exception would be if this is intricately coupled to the notion of the
chrome
> > browser app. But even if there are some linkages here, is it possible this
> could
> > stand alone and be picked up & configured by src/chrome?
> 
> It's very possible for this to stand alone and live in say
src/win8/metro_driver
> instead. There's a case for it to live both under chrome and not, and I don't
> feel strongly either way.
> 
> The directory structure here was from a mild consensus we got from an earlier
> mail thread we had (titled "Namespace(s) for Metro code?"), but I have no
> objection to moving metro_driver out from under /chrome if you'd prefer. What
> say you?

[grt (UTC plus 2), 2012-08-29 02:59:18]

https://chromiumcodereview.appspot.com/10875008/diff/14001/chrome/browser/ui/...
File chrome/browser/ui/metro_driver/metro_driver.gyp (right):

https://chromiumcodereview.appspot.com/10875008/diff/14001/chrome/browser/ui/...
chrome/browser/ui/metro_driver/metro_driver.gyp:30: 'target_name':
'check_sdk_patch_metro_driver',
is it appropriate to move this target into chrome.gyp, and have metro_driver and
delegegate_execute take a dependency on it?  i think that'd avoid the
duplication.  i'm not sure why that'd be a bad thing.

https://chromiumcodereview.appspot.com/10875008/diff/14001/chrome/tools/build...
File chrome/tools/build/win/check_sdk_patch.py (right):

https://chromiumcodereview.appspot.com/10875008/diff/14001/chrome/tools/build...
chrome/tools/build/win/check_sdk_patch.py:21: return -1
are you following a convention for using negative values for error results?  in
the unix world, the exit code is effectively a uint8.  i'd rather see just 1 and
2 rather than -1 and -2, but perhaps i'm ignorant.

[robertshield, 2012-08-30 13:34:05]

https://chromiumcodereview.appspot.com/10875008/diff/14001/chrome/browser/ui/...
File chrome/browser/ui/metro_driver/metro_driver.gyp (right):

https://chromiumcodereview.appspot.com/10875008/diff/14001/chrome/browser/ui/...
chrome/browser/ui/metro_driver/metro_driver.gyp:30: 'target_name':
'check_sdk_patch_metro_driver',
On 2012/08/29 02:59:18, grt wrote:
> is it appropriate to move this target into chrome.gyp, and have metro_driver
and
> delegegate_execute take a dependency on it?  i think that'd avoid the
> duplication.  i'm not sure why that'd be a bad thing.

I moved it into a common target under src/win8. Seemed too win8 specific and
chrome.gyp is already a tangled nest of complicated platform-specific stuff.

https://chromiumcodereview.appspot.com/10875008/diff/14001/chrome/tools/build...
File chrome/tools/build/win/check_sdk_patch.py (right):

https://chromiumcodereview.appspot.com/10875008/diff/14001/chrome/tools/build...
chrome/tools/build/win/check_sdk_patch.py:21: return -1
On 2012/08/29 02:59:18, grt wrote:
> are you following a convention for using negative values for error results? 
in
> the unix world, the exit code is effectively a uint8.  i'd rather see just 1
and
> 2 rather than -1 and -2, but perhaps i'm ignorant.

Done.

[grt (UTC plus 2), 2012-08-30 14:51:43]

lgtm

[robertshield, 2012-08-31 03:17:16]

On 2012/08/30 14:51:43, grt wrote:
> lgtm

Ben, updated paths here such that metro_driver lives under src/win8. PTAL

[Ben Goodger (Google), 2012-08-31 19:46:25]

lgtm

[commit-bot: I haz the power, 2012-09-07 14:39:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/robertshield@chromium.org/10875008/32001

[commit-bot: I haz the power, 2012-09-07 14:40:07]

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

[commit-bot: I haz the power, 2012-09-07 15:39:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/robertshield@chromium.org/10875008/30003

[commit-bot: I haz the power, 2012-09-07 15:39:44]

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

[commit-bot: I haz the power, 2012-09-07 16:05:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/robertshield@chromium.org/10875008/37001

[commit-bot: I haz the power, 2012-09-07 18:24:54]

Change committed as 155429