Remove dependency of chrome in GuestViewBase.

Remove dependency of chrome in GuestViewBase.

Use ExtensionsAPIClient to register guests.

BUG=352290
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=288427

[Fady Samuel, 2014-08-07 20:10:48]

https://codereview.chromium.org/444113003/diff/20001/chrome/browser/chrome_co...
File chrome/browser/chrome_content_browser_client.h (right):

https://codereview.chromium.org/444113003/diff/20001/chrome/browser/chrome_co...
chrome/browser/chrome_content_browser_client.h:334: static void
GetGuestViewDefaultContentSettingRules(
This probably doesn't need to be here at all. You can just stick it in an
anonymous namespace in the cc file.

https://codereview.chromium.org/444113003/diff/20001/chrome/browser/guest_vie...
File chrome/browser/guest_view/guest_view_base.cc (right):

https://codereview.chromium.org/444113003/diff/20001/chrome/browser/guest_vie...
chrome/browser/guest_view/guest_view_base.cc:424:
extensions::ExtensionsAPIClient::Get()->OnRegisterGuestViewType();
Rename OnRegisterGuestViewType to RegisterGuestViewTypes();

[Xi Han, 2014-08-07 20:36:07]

https://codereview.chromium.org/444113003/diff/20001/chrome/browser/chrome_co...
File chrome/browser/chrome_content_browser_client.h (right):

https://codereview.chromium.org/444113003/diff/20001/chrome/browser/chrome_co...
chrome/browser/chrome_content_browser_client.h:334: static void
GetGuestViewDefaultContentSettingRules(
On 2014/08/07 20:10:48, Fady Samuel wrote:
> This probably doesn't need to be here at all. You can just stick it in an
> anonymous namespace in the cc file.

Done.

https://codereview.chromium.org/444113003/diff/20001/chrome/browser/guest_vie...
File chrome/browser/guest_view/guest_view_base.cc (right):

https://codereview.chromium.org/444113003/diff/20001/chrome/browser/guest_vie...
chrome/browser/guest_view/guest_view_base.cc:424:
extensions::ExtensionsAPIClient::Get()->OnRegisterGuestViewType();
On 2014/08/07 20:10:48, Fady Samuel wrote:
> Rename OnRegisterGuestViewType to RegisterGuestViewTypes();

Done.

[Xi Han, 2014-08-07 21:12:21]

rockot@chromium.org: Please review changes in 

chrome/browser/extensions/api/chrome_extensions_api_client.h
chrome/browser/extensions/api/chrome_extensions_api_client.cc
extensions/browser/api/extensions_api_client.h

Thanks!

[Ken Rockot(use gerrit already), 2014-08-07 22:37:41]

https://codereview.chromium.org/444113003/diff/60001/chrome/browser/extension...
File chrome/browser/extensions/api/chrome_extensions_api_client.cc (right):

https://codereview.chromium.org/444113003/diff/60001/chrome/browser/extension...
chrome/browser/extensions/api/chrome_extensions_api_client.cc:76:
AppViewGuest::Register();
Please also wrap these Register() calls in an #if defined(ENABLE_EXTENSIONS) as
well

https://codereview.chromium.org/444113003/diff/60001/extensions/browser/api/e...
File extensions/browser/api/extensions_api_client.h (right):

https://codereview.chromium.org/444113003/diff/60001/extensions/browser/api/e...
extensions/browser/api/extensions_api_client.h:74: virtual void
RegisterGuestViewType() {}
nit: Should probably pluralize this, i.e. RegisterGuestViewTypes

[Xi Han, 2014-08-08 14:12:12]

Ken: would you please take a look at the chrome_extensions_api_client.cc again,
whether we should remove all the #if defined(ENABLE_EXTENSIONS) or we need to
update other functions (by adding #if defined) in this class for code
consistency?

https://codereview.chromium.org/444113003/diff/60001/chrome/browser/extension...
File chrome/browser/extensions/api/chrome_extensions_api_client.cc (right):

https://codereview.chromium.org/444113003/diff/60001/chrome/browser/extension...
chrome/browser/extensions/api/chrome_extensions_api_client.cc:76:
AppViewGuest::Register();
On 2014/08/07 22:37:40, Ken Rockot wrote:
> Please also wrap these Register() calls in an #if defined(ENABLE_EXTENSIONS)
as
> well

Wait a second. It is safe to add #if defined(ENABLE_EXTENSIONS), but I begin to
think about whether it is necessary in this class (also the #if defined that I
added in the "includes"). This class is called ChromeExtensionsAPIClient, can we
assume that extensions is enable here?

https://codereview.chromium.org/444113003/diff/60001/extensions/browser/api/e...
File extensions/browser/api/extensions_api_client.h (right):

https://codereview.chromium.org/444113003/diff/60001/extensions/browser/api/e...
extensions/browser/api/extensions_api_client.h:74: virtual void
RegisterGuestViewType() {}
On 2014/08/07 22:37:40, Ken Rockot wrote:
> nit: Should probably pluralize this, i.e. RegisterGuestViewTypes

Done.

[Ken Rockot(use gerrit already), 2014-08-08 15:31:13]

On 2014/08/08 14:12:12, hanxi wrote:
> Ken: would you please take a look at the chrome_extensions_api_client.cc
again,
> whether we should remove all the #if defined(ENABLE_EXTENSIONS) or we need to
> update other functions (by adding #if defined) in this class for code
> consistency?
> 
>
https://codereview.chromium.org/444113003/diff/60001/chrome/browser/extension...
> File chrome/browser/extensions/api/chrome_extensions_api_client.cc (right):
> 
>
https://codereview.chromium.org/444113003/diff/60001/chrome/browser/extension...
> chrome/browser/extensions/api/chrome_extensions_api_client.cc:76:
> AppViewGuest::Register();
> On 2014/08/07 22:37:40, Ken Rockot wrote:
> > Please also wrap these Register() calls in an #if defined(ENABLE_EXTENSIONS)
> as
> > well
> 
> Wait a second. It is safe to add #if defined(ENABLE_EXTENSIONS), but I begin
to
> think about whether it is necessary in this class (also the #if defined that I
> added in the "includes"). This class is called ChromeExtensionsAPIClient, can
we
> assume that extensions is enable here?
> 
>
https://codereview.chromium.org/444113003/diff/60001/extensions/browser/api/e...
> File extensions/browser/api/extensions_api_client.h (right):
> 
>
https://codereview.chromium.org/444113003/diff/60001/extensions/browser/api/e...
> extensions/browser/api/extensions_api_client.h:74: virtual void
> RegisterGuestViewType() {}
> On 2014/08/07 22:37:40, Ken Rockot wrote:
> > nit: Should probably pluralize this, i.e. RegisterGuestViewTypes
> 
> Done.

Ah! You are correct. The checks are not needed. I think this file was being
compiled
on Android at one point, but that is no longer the case.

LGTM

[Fady Samuel, 2014-08-08 17:39:56]

lgtm

[Fady Samuel, 2014-08-08 17:39:57]

lgtm

lgtm

[Xi Han, 2014-08-08 17:57:06]

The CQ bit was checked by hanxi@chromium.org

[commit-bot: I haz the power, 2014-08-08 17:58:44]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hanxi@chromium.org/444113003/100001

[commit-bot: I haz the power, 2014-08-08 21:02:33]

Change committed as 288427