Chromium Code Reviews
[email protected] (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(377)

Issue 10872002: beginnings of metro viewer process (Closed)

Created:
8 years, 4 months ago by scottmg
Modified:
8 years, 3 months ago
CC:
chromium-reviews, darin-cc_chromium.org, jam, cpu_(ooo_6.6-7.5)
Visibility:
Public.

Description

Bunch of boilerplate to set up a process, ipc, etc. Only hooked up in "aura_demo --viewer" right now, and doesn't actually accomplish anything other than opening a window yet. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=157367

Patch Set 1 #

Patch Set 2 : add basic window creation for testing in non-metro #

Patch Set 3 : . #

Patch Set 4 : move to ui and make toplevel an aura window #

Patch Set 5 : ctor order #

Patch Set 6 : linux fix #

Total comments: 12

Patch Set 7 : cleanup #

Total comments: 3

Patch Set 8 : . #

Unified diffs Side-by-side diffs Delta from patch set Stats (+468 lines, -49 lines) Patch
M ipc/ipc_message_utils.h View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M ui/aura/aura.gyp View 1 2 3 4 5 6 1 chunk +2 lines, -0 lines 0 comments Download
M ui/aura/demo/demo_main.cc View 1 2 3 4 5 6 3 chunks +27 lines, -12 lines 0 comments Download
M ui/aura/root_window.cc View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M ui/aura/root_window_host_delegate.h View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
A ui/viewer/viewer.gyp View 1 2 3 4 5 6 1 chunk +34 lines, -0 lines 0 comments Download
A ui/viewer/viewer_host_win.h View 1 2 3 4 5 6 7 1 chunk +36 lines, -0 lines 0 comments Download
A ui/viewer/viewer_host_win.cc View 1 2 3 1 chunk +53 lines, -0 lines 0 comments Download
A + ui/viewer/viewer_ipc_server.h View 1 2 3 4 5 6 3 chunks +11 lines, -28 lines 0 comments Download
A ui/viewer/viewer_ipc_server.cc View 1 2 3 4 1 chunk +98 lines, -0 lines 0 comments Download
A ui/viewer/viewer_main.cc View 1 2 3 4 5 6 1 chunk +34 lines, -0 lines 0 comments Download
A + ui/viewer/viewer_message_generator.h View 1 2 3 4 5 6 1 chunk +1 line, -2 lines 0 comments Download
A + ui/viewer/viewer_message_generator.cc View 1 2 3 4 5 6 1 chunk +6 lines, -7 lines 0 comments Download
A ui/viewer/viewer_messages.h View 1 2 3 1 chunk +26 lines, -0 lines 0 comments Download
A ui/viewer/viewer_process.h View 1 2 3 4 5 6 1 chunk +69 lines, -0 lines 0 comments Download
A ui/viewer/viewer_process.cc View 1 2 3 4 5 6 1 chunk +67 lines, -0 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
scottmg
fyi. No idea if any of this is sensible until I figure out what's going ...
8 years, 4 months ago (2012-08-23 18:10:31 UTC) #1
scottmg
PTAL when you have a chance. Planning to add the host side connection next, and ...
8 years, 3 months ago (2012-08-27 22:59:16 UTC) #2
Ben Goodger (Google)
Cool! LGTM.
8 years, 3 months ago (2012-08-27 23:28:41 UTC) #3
tfarina
http://codereview.chromium.org/10872002/diff/10003/ui/ui.gyp File ui/ui.gyp (right): http://codereview.chromium.org/10872002/diff/10003/ui/ui.gyp#newcode775 ui/ui.gyp:775: 'target_name': 'viewer', nonono, please, like ui/app_list, ui/views, and what ...
8 years, 3 months ago (2012-08-28 00:54:22 UTC) #4
tfarina
http://codereview.chromium.org/10872002/diff/10003/ui/viewer/viewer_main.cc File ui/viewer/viewer_main.cc (right): http://codereview.chromium.org/10872002/diff/10003/ui/viewer/viewer_main.cc#newcode18 ui/viewer/viewer_main.cc:18: // TODO(scottmg): Constant. why don't you do this now? ...
8 years, 3 months ago (2012-08-28 00:55:36 UTC) #5
tfarina
http://codereview.chromium.org/10872002/diff/10003/ui/viewer/viewer_ipc_server.h File ui/viewer/viewer_ipc_server.h (right): http://codereview.chromium.org/10872002/diff/10003/ui/viewer/viewer_ipc_server.h#newcode35 ui/viewer/viewer_ipc_server.h:35: rm this extra blank line.
8 years, 3 months ago (2012-08-28 01:14:15 UTC) #6
tfarina
http://codereview.chromium.org/10872002/diff/10003/ui/viewer/viewer_host_win.h File ui/viewer/viewer_host_win.h (right): http://codereview.chromium.org/10872002/diff/10003/ui/viewer/viewer_host_win.h#newcode9 ui/viewer/viewer_host_win.h:9: #include "ui/aura/root_window.h" why can't you forward declare this? http://codereview.chromium.org/10872002/diff/10003/ui/viewer/viewer_host_win.h#newcode15 ...
8 years, 3 months ago (2012-08-28 01:15:51 UTC) #7
tfarina
http://codereview.chromium.org/10872002/diff/10003/ui/aura/demo/demo_main.cc File ui/aura/demo/demo_main.cc (right): http://codereview.chromium.org/10872002/diff/10003/ui/aura/demo/demo_main.cc#newcode181 ui/aura/demo/demo_main.cc:181: ResourceBundle::InitSharedInstanceWithLocale("en-US", NULL); ui::ResourceBundle
8 years, 3 months ago (2012-08-28 01:17:08 UTC) #8
tfarina
http://codereview.chromium.org/10872002/diff/10003/ui/viewer/viewer_process.cc File ui/viewer/viewer_process.cc (right): http://codereview.chromium.org/10872002/diff/10003/ui/viewer/viewer_process.cc#newcode20 ui/viewer/viewer_process.cc:20: virtual void CleanUp(); OVERRIDE? http://codereview.chromium.org/10872002/diff/10003/ui/viewer/viewer_process.cc#newcode27 ui/viewer/viewer_process.cc:27: ViewerIOThread::~ViewerIOThread() { can ...
8 years, 3 months ago (2012-08-28 01:19:45 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/[email protected]/10872002/18001
8 years, 3 months ago (2012-09-17 23:32:32 UTC) #10
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build. Your ...
8 years, 3 months ago (2012-09-17 23:57:36 UTC) #11
tfarina
did you fix the other nits I posted? https://chromiumcodereview.appspot.com/10872002/diff/18001/ui/viewer/viewer_host_win.h File ui/viewer/viewer_host_win.h (right): https://chromiumcodereview.appspot.com/10872002/diff/18001/ui/viewer/viewer_host_win.h#newcode6 ui/viewer/viewer_host_win.h:6: #define ...
8 years, 3 months ago (2012-09-18 01:11:52 UTC) #12
scottmg
8 years, 3 months ago (2012-09-18 03:09:36 UTC) #13
On 2012/09/18 01:11:52, tfarina wrote:
> did you fix the other nits I posted?

yes, those that were still applicable, except for the namespace.

> 
>
https://chromiumcodereview.appspot.com/10872002/diff/18001/ui/viewer/viewer_h...
> File ui/viewer/viewer_host_win.h (right):
> 
>
https://chromiumcodereview.appspot.com/10872002/diff/18001/ui/viewer/viewer_h...
> ui/viewer/viewer_host_win.h:6: #define UI_VIEWER_VIEWER_VIEWER_HOST_WIN_H_
> nit: UI_VIEWER_VIEWER_HOST_WIN_H_
> 
> there is an extra VIEWER here.

done.

> 
>
https://chromiumcodereview.appspot.com/10872002/diff/18001/ui/viewer/viewer_h...
> ui/viewer/viewer_host_win.h:12: namespace aura { class RootWindow; }
> ham? what is that :)
> 
> I think you meant:

ok.

> 
> namespace aura {
> class FocusManager;
> class RootWindow;
> 
> namespace shared {
> class RootWindowCaptureClient;
> }
> 
> }
> 
> Or did you do this way to save vertical lines?
> 
>
https://chromiumcodereview.appspot.com/10872002/diff/18001/ui/viewer/viewer_h...
> ui/viewer/viewer_host_win.h:15: class ViewerHostWin {
> doesn't this needs to be in ui namespace? or even ui::viewer::, whatever...

I don't know. This code is most closely associated with ui/surface, and that
doesn't have any namespaces. This will probably have to move to chrome
relatively soon anyway.

Powered by Google App Engine
This is Rietveld 408576698