Towards Decoupling GuestView from Extensions

Towards Decoupling GuestView from Extensions

This CL moves GuestViewEvent to its own file.

It moves the permission check to GuestViewManager.

GuestViewBase now no longer directly depends on extensions.

BUG=444869
TBR=rockot@chromium.org for extensions.gypi

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

[Fady Samuel, 2015-04-15 02:18:24]

fsamuel@chromium.org changed reviewers:
+ lazyboy@chromium.org

[lazyboy, 2015-04-15 02:50:11]

lgtm % nits/comments

https://chromiumcodereview.appspot.com/1087123002/diff/1/extensions/browser/g...
File extensions/browser/guest_view/guest_view_event.h (right):

https://chromiumcodereview.appspot.com/1087123002/diff/1/extensions/browser/g...
extensions/browser/guest_view/guest_view_event.h:17: class GuestViewEvent {
Description of this class.

https://chromiumcodereview.appspot.com/1087123002/diff/1/extensions/browser/g...
extensions/browser/guest_view/guest_view_event.h:24: void Dispatch(int
instance_id);
Description, also note that this is self destructing.

https://chromiumcodereview.appspot.com/1087123002/diff/1/extensions/browser/g...
extensions/browser/guest_view/guest_view_event.h:28: GuestViewBase* const
guest_;
Note lifetime of |this| vs |guest_| and why accessing |guest_| is safe from
here.

https://chromiumcodereview.appspot.com/1087123002/diff/1/extensions/browser/g...
extensions/browser/guest_view/guest_view_event.h:30:
scoped_ptr<base::DictionaryValue> args_;
DISALLOW_COPY_AND_ASSIGN

https://chromiumcodereview.appspot.com/1087123002/diff/1/extensions/browser/g...
File extensions/browser/guest_view/guest_view_manager.cc (right):

https://chromiumcodereview.appspot.com/1087123002/diff/1/extensions/browser/g...
extensions/browser/guest_view/guest_view_manager.cc:237: GuestViewBase* guest,
run git cl format for the CL, these probably can fit in one line.

https://chromiumcodereview.appspot.com/1087123002/diff/1/extensions/browser/g...
File extensions/browser/guest_view/guest_view_manager.h (right):

https://chromiumcodereview.appspot.com/1087123002/diff/1/extensions/browser/g...
extensions/browser/guest_view/guest_view_manager.h:107: bool
IsGuestAvailableToContext(GuestViewBase* guest,
Need a description line for the public function.

https://chromiumcodereview.appspot.com/1087123002/diff/1/extensions/extension...
File extensions/extensions.gypi (right):

https://chromiumcodereview.appspot.com/1087123002/diff/1/extensions/extension...
extensions/extensions.gypi:647: 'browser/guest_view/guest_view_event.cc',
sort

[Fady Samuel, 2015-04-15 15:14:04]

fsamuel@chromium.org changed reviewers:
+ rockot@chromium.org

[Fady Samuel, 2015-04-15 15:14:05]

+rockot@ for extensions.gypi signin off. Thanks!

https://codereview.chromium.org/1087123002/diff/1/extensions/browser/guest_vi...
File extensions/browser/guest_view/guest_view_event.h (right):

https://codereview.chromium.org/1087123002/diff/1/extensions/browser/guest_vi...
extensions/browser/guest_view/guest_view_event.h:17: class GuestViewEvent {
On 2015/04/15 02:50:11, lazyboy wrote:
> Description of this class.

Done.

https://codereview.chromium.org/1087123002/diff/1/extensions/browser/guest_vi...
extensions/browser/guest_view/guest_view_event.h:24: void Dispatch(int
instance_id);
On 2015/04/15 02:50:11, lazyboy wrote:
> Description, also note that this is self destructing.

Done.

https://codereview.chromium.org/1087123002/diff/1/extensions/browser/guest_vi...
extensions/browser/guest_view/guest_view_event.h:28: GuestViewBase* const
guest_;
On 2015/04/15 02:50:11, lazyboy wrote:
> Note lifetime of |this| vs |guest_| and why accessing |guest_| is safe from
> here.

Done.

https://codereview.chromium.org/1087123002/diff/1/extensions/browser/guest_vi...
extensions/browser/guest_view/guest_view_event.h:30:
scoped_ptr<base::DictionaryValue> args_;
On 2015/04/15 02:50:11, lazyboy wrote:
> DISALLOW_COPY_AND_ASSIGN

Done.

https://codereview.chromium.org/1087123002/diff/1/extensions/browser/guest_vi...
File extensions/browser/guest_view/guest_view_manager.cc (right):

https://codereview.chromium.org/1087123002/diff/1/extensions/browser/guest_vi...
extensions/browser/guest_view/guest_view_manager.cc:237: GuestViewBase* guest,
On 2015/04/15 02:50:11, lazyboy wrote:
> run git cl format for the CL, these probably can fit in one line.

Nope.

https://codereview.chromium.org/1087123002/diff/1/extensions/browser/guest_vi...
File extensions/browser/guest_view/guest_view_manager.h (right):

https://codereview.chromium.org/1087123002/diff/1/extensions/browser/guest_vi...
extensions/browser/guest_view/guest_view_manager.h:107: bool
IsGuestAvailableToContext(GuestViewBase* guest,
On 2015/04/15 02:50:11, lazyboy wrote:
> Need a description line for the public function.

Done.

https://codereview.chromium.org/1087123002/diff/1/extensions/extensions.gypi
File extensions/extensions.gypi (right):

https://codereview.chromium.org/1087123002/diff/1/extensions/extensions.gypi#...
extensions/extensions.gypi:647: 'browser/guest_view/guest_view_event.cc',
On 2015/04/15 02:50:11, lazyboy wrote:
> sort

Done.

[Fady Samuel, 2015-04-15 16:56:11]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2015-04-15 16:56:12]

The patchset sent to the CQ was uploaded after l-g-t-m from lazyboy@chromium.org
Link to the patchset: https://codereview.chromium.org/1087123002/#ps20001
(title: "Addressed lazyboy@'s comments. Removed unnecessary includes. Simplified
GuestViewEvent.")

[commit-bot: I haz the power, 2015-04-15 16:56:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1087123002/20001

[commit-bot: I haz the power, 2015-04-15 18:57:21]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-04-15 18:59:24]

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