[Ozone-GBM] Pumb DriWindowDelegate throughout the platform

[Ozone-GBM] Pumb DriWindowDelegate throughout the platform

BUG=392478
NOTRY=true

Committed: https://crrev.com/d3123eb89cdb637bb5869126ac6fc2f5bb098766
Cr-Commit-Position: refs/heads/master@{#291715}

[dnicoara, 2014-08-20 20:58:38]

This is the second part of the DriWindowDelegate wiring (links the delegate to
the DriWindow and the Surface impls).

[alexst (slow to review), 2014-08-20 21:51:09]

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/d...
File ui/ozone/platform/dri/dri_surface_unittest.cc (right):

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/d...
ui/ozone/platform/dri/dri_surface_unittest.cc:31: scoped_ptr<ui::CrtcState>(
nit: I think using make_scoped_ptr() is a bit cleaner here.

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/d...
File ui/ozone/platform/dri/dri_vsync_provider.h (right):

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/d...
ui/ozone/platform/dri/dri_vsync_provider.h:22: DriWindowDelegate*
window_delegate_;
// not owned.

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/d...
File ui/ozone/platform/dri/dri_window.cc (right):

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/d...
ui/ozone/platform/dri/dri_window.cc:25: dri_window_delegate_->Initialize();
My enemy, constructor workload is back! :)

Can we put an init in ozone_platform_dri.cc?

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/g...
File ui/ozone/platform/dri/gbm_surface_factory.cc (right):

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/g...
ui/ozone/platform/dri/gbm_surface_factory.cc:121: DriWindowDelegate* delegate =
GetOrCreateWindowDelegate(widget);
I think this leaks. As far as I can tell looking through code, this is a raw
pointer everywhere.

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/o...
File ui/ozone/platform/dri/ozone_platform_dri.cc (right):

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/o...
ui/ozone/platform/dri/ozone_platform_dri.cc:70:
scoped_ptr<DriWindowDelegate>(new DriWindowDelegateImpl(
make_scoped_ptr()

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/o...
File ui/ozone/platform/dri/ozone_platform_gbm.cc (right):

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/o...
ui/ozone/platform/dri/ozone_platform_gbm.cc:104:
scoped_ptr<DriWindowDelegate>(new DriWindowDelegateProxy(
make_scoped_ptr

[dnicoara, 2014-08-21 20:54:17]

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/d...
File ui/ozone/platform/dri/dri_surface_unittest.cc (right):

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/d...
ui/ozone/platform/dri/dri_surface_unittest.cc:31: scoped_ptr<ui::CrtcState>(
On 2014/08/20 21:51:09, alexst wrote:
> nit: I think using make_scoped_ptr() is a bit cleaner here.

Done.

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/d...
File ui/ozone/platform/dri/dri_vsync_provider.h (right):

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/d...
ui/ozone/platform/dri/dri_vsync_provider.h:22: DriWindowDelegate*
window_delegate_;
On 2014/08/20 21:51:09, alexst wrote:
> // not owned.

Done.

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/d...
File ui/ozone/platform/dri/dri_window.cc (right):

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/d...
ui/ozone/platform/dri/dri_window.cc:25: dri_window_delegate_->Initialize();
On 2014/08/20 21:51:09, alexst wrote:
> My enemy, constructor workload is back! :)
> 
> Can we put an init in ozone_platform_dri.cc?

I no longer care and I just want this to go in so I made the change ... but it
is in protest since I still think this is wrong for the following reasons:
1) Layering violation. One should not care what happens in objects unless they
return errors (none do in this case)
2) No calls return any errors (so doesn't break the style guide)
3) Code verbosity is always bad. Now there are multiple places that need to call
a "void Initialize" right after construction. The initializer doesn't add any
extra value (equivalent to doing it in the constructor).

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/g...
File ui/ozone/platform/dri/gbm_surface_factory.cc (right):

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/g...
ui/ozone/platform/dri/gbm_surface_factory.cc:121: DriWindowDelegate* delegate =
GetOrCreateWindowDelegate(widget);
On 2014/08/20 21:51:09, alexst wrote:
> I think this leaks. As far as I can tell looking through code, this is a raw
> pointer everywhere.

Arr, yes, I forgot about this through all the refactoring ... I've inverted the
ownership for the sake of simplicity, but this is going to haunt us in the
future.

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/o...
File ui/ozone/platform/dri/ozone_platform_dri.cc (right):

https://codereview.chromium.org/469343003/diff/100001/ui/ozone/platform/dri/o...
ui/ozone/platform/dri/ozone_platform_dri.cc:70:
scoped_ptr<DriWindowDelegate>(new DriWindowDelegateImpl(
On 2014/08/20 21:51:09, alexst wrote:
> make_scoped_ptr()

"I'm sorry dave i'm afraid i can't do that"

Need to return a scoped_ptr for the interface and make_scoped_ptr returns a
scoped_ptr for the implementation.

[alexst (slow to review), 2014-08-21 21:43:36]

lgtm.

[dnicoara, 2014-08-25 18:06:14]

The CQ bit was checked by dnicoara@chromium.org

[commit-bot: I haz the power, 2014-08-25 18:07:05]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dnicoara@chromium.org/469343003/140001

[commit-bot: I haz the power, 2014-08-25 18:08:28]

Committed patchset #8 (140001) as 31d3b10338e665736c61fe3a807095a63c862a7e

[commit-bot: I haz the power, 2014-09-10 02:35:43]

Patchset 8 (id:??) landed as
https://crrev.com/d3123eb89cdb637bb5869126ac6fc2f5bb098766
Cr-Commit-Position: refs/heads/master@{#291715}