Refactor ExtensionOptionsGuestDelegate to have an ExtensionOptionsGuest

Refactor ExtensionOptionsGuestDelegate to have an ExtensionOptionsGuest

Give the delegate class an ExtensionOptionsGuest member so that the
calls to the delegate do not require passing a WebContents.

BUG=409316
Committed: https://crrev.com/b34659d0a0acb3815169b748a5d8cb00b24fa160
Cr-Commit-Position: refs/heads/master@{#294495}

[ericzeng, 2014-09-11 18:44:45]

ericzeng@chromium.org changed reviewers:
+ fsamuel@chromium.org, rdevlin.cronin@chromium.org, yoz@chromium.org

[ericzeng, 2014-09-11 18:44:45]

PTAL

[Fady Samuel, 2014-09-11 18:53:36]

lgtm with nits.

https://codereview.chromium.org/562353002/diff/1/chrome/browser/guest_view/ex...
File
chrome/browser/guest_view/extension_options/chrome_extension_options_guest_delegate.cc
(right):

https://codereview.chromium.org/562353002/diff/1/chrome/browser/guest_view/ex...
chrome/browser/guest_view/extension_options/chrome_extension_options_guest_delegate.cc:24:
CreateChromeExtensionWebContentsObserver() {
Call this DidInitialize

https://codereview.chromium.org/562353002/diff/1/extensions/browser/guest_vie...
File extensions/browser/guest_view/extension_options/extension_options_guest.cc
(right):

https://codereview.chromium.org/562353002/diff/1/extensions/browser/guest_vie...
extensions/browser/guest_view/extension_options/extension_options_guest.cc:121:
->CreateChromeExtensionWebContentsObserver();
I would just call it DidInitialize, otherwise you're exposing chrome module
details to extensions.

https://codereview.chromium.org/562353002/diff/1/extensions/browser/guest_vie...
File
extensions/browser/guest_view/extension_options/extension_options_guest_delegate.h
(right):

https://codereview.chromium.org/562353002/diff/1/extensions/browser/guest_vie...
extensions/browser/guest_view/extension_options/extension_options_guest_delegate.h:27:
virtual void CreateChromeExtensionWebContentsObserver() = 0;
DidInitialize.

[ericzeng, 2014-09-11 19:00:05]

On 2014/09/11 18:53:36, Fady Samuel wrote:
> lgtm with nits.
> 
>
https://codereview.chromium.org/562353002/diff/1/chrome/browser/guest_view/ex...
> File
>
chrome/browser/guest_view/extension_options/chrome_extension_options_guest_delegate.cc
> (right):
> 
>
https://codereview.chromium.org/562353002/diff/1/chrome/browser/guest_view/ex...
>
chrome/browser/guest_view/extension_options/chrome_extension_options_guest_delegate.cc:24:
> CreateChromeExtensionWebContentsObserver() {
> Call this DidInitialize
> 
>
https://codereview.chromium.org/562353002/diff/1/extensions/browser/guest_vie...
> File
extensions/browser/guest_view/extension_options/extension_options_guest.cc
> (right):
> 
>
https://codereview.chromium.org/562353002/diff/1/extensions/browser/guest_vie...
>
extensions/browser/guest_view/extension_options/extension_options_guest.cc:121:
> ->CreateChromeExtensionWebContentsObserver();
> I would just call it DidInitialize, otherwise you're exposing chrome module
> details to extensions.
> 
>
https://codereview.chromium.org/562353002/diff/1/extensions/browser/guest_vie...
> File
>
extensions/browser/guest_view/extension_options/extension_options_guest_delegate.h
> (right):
> 
>
https://codereview.chromium.org/562353002/diff/1/extensions/browser/guest_vie...
>
extensions/browser/guest_view/extension_options/extension_options_guest_delegate.h:27:
> virtual void CreateChromeExtensionWebContentsObserver() = 0;
> DidInitialize.

Done.

[Yoyo Zhou, 2014-09-11 20:17:26]

LGTM with nits

https://chromiumcodereview.appspot.com/562353002/diff/20001/chrome/browser/gu...
File
chrome/browser/guest_view/extension_options/chrome_extension_options_guest_delegate.h
(right):

https://chromiumcodereview.appspot.com/562353002/diff/20001/chrome/browser/gu...
chrome/browser/guest_view/extension_options/chrome_extension_options_guest_delegate.h:11:
#include
"extensions/browser/guest_view/extension_options/extension_options_guest.h"
Can you forward declare ExtensionOptionsGuest?

https://chromiumcodereview.appspot.com/562353002/diff/20001/extensions/browse...
File
extensions/browser/guest_view/extension_options/extension_options_guest_delegate.cc
(right):

https://chromiumcodereview.appspot.com/562353002/diff/20001/extensions/browse...
extensions/browser/guest_view/extension_options/extension_options_guest_delegate.cc:7:
namespace extensions {
nit: newline after

https://chromiumcodereview.appspot.com/562353002/diff/20001/extensions/browse...
File
extensions/browser/guest_view/extension_options/extension_options_guest_delegate.h
(right):

https://chromiumcodereview.appspot.com/562353002/diff/20001/extensions/browse...
extensions/browser/guest_view/extension_options/extension_options_guest_delegate.h:27:
virtual void DidInitialize() = 0;
Comment that this is called from ExtensionOptionsGuest::DidInitialize

[ericzeng, 2014-09-11 20:26:00]

On 2014/09/11 20:17:26, Yoyo Zhou wrote:
> LGTM with nits
> 
>
https://chromiumcodereview.appspot.com/562353002/diff/20001/chrome/browser/gu...
> File
>
chrome/browser/guest_view/extension_options/chrome_extension_options_guest_delegate.h
> (right):
> 
>
https://chromiumcodereview.appspot.com/562353002/diff/20001/chrome/browser/gu...
>
chrome/browser/guest_view/extension_options/chrome_extension_options_guest_delegate.h:11:
> #include
> "extensions/browser/guest_view/extension_options/extension_options_guest.h"
> Can you forward declare ExtensionOptionsGuest?
> 
>
https://chromiumcodereview.appspot.com/562353002/diff/20001/extensions/browse...
> File
>
extensions/browser/guest_view/extension_options/extension_options_guest_delegate.cc
> (right):
> 
>
https://chromiumcodereview.appspot.com/562353002/diff/20001/extensions/browse...
>
extensions/browser/guest_view/extension_options/extension_options_guest_delegate.cc:7:
> namespace extensions {
> nit: newline after
> 
>
https://chromiumcodereview.appspot.com/562353002/diff/20001/extensions/browse...
> File
>
extensions/browser/guest_view/extension_options/extension_options_guest_delegate.h
> (right):
> 
>
https://chromiumcodereview.appspot.com/562353002/diff/20001/extensions/browse...
>
extensions/browser/guest_view/extension_options/extension_options_guest_delegate.h:27:
> virtual void DidInitialize() = 0;
> Comment that this is called from ExtensionOptionsGuest::DidInitialize

Done.

[ericzeng, 2014-09-11 20:26:05]

The CQ bit was checked by ericzeng@chromium.org

[commit-bot: I haz the power, 2014-09-11 20:30:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/562353002/40001

[commit-bot: I haz the power, 2014-09-11 23:49:04]

Committed patchset #3 (id:40001) as bf718252b6e272660a1298e527fe98e6a20a9754

[commit-bot: I haz the power, 2014-09-12 00:22:14]

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