Moving web_view.js to extensions.

Moving web_view.js to extensions.

BUG=352293
Committed: https://crrev.com/a71ea760fdfc9ca27ce29e160fb393c961555465
Cr-Commit-Position: refs/heads/master@{#294519}

[lfg, 2014-09-11 19:14:39]

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

[Fady Samuel, 2014-09-11 19:24:03]

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

[Fady Samuel, 2014-09-11 19:24:03]

https://codereview.chromium.org/564913003/diff/1/chrome/renderer/extensions/c...
File chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc (left):

https://codereview.chromium.org/564913003/diff/1/chrome/renderer/extensions/c...
chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc:277: if
(context->GetAvailability("webViewInternal").is_available()) {
This check should be done in extensions as well.

[lfg, 2014-09-11 19:27:59]

https://codereview.chromium.org/564913003/diff/1/chrome/renderer/extensions/c...
File chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc (left):

https://codereview.chromium.org/564913003/diff/1/chrome/renderer/extensions/c...
chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc:277: if
(context->GetAvailability("webViewInternal").is_available()) {
On 2014/09/11 19:24:03, Fady Samuel wrote:
> This check should be done in extensions as well.

I'll do it in a future patch. This patch only moves web_view.js to extensions,
it still doesn't expose it in extensions (i.e. module_system->Require("webView")
still isn't called from extensions).

[Fady Samuel, 2014-09-11 19:31:40]

https://codereview.chromium.org/564913003/diff/1/chrome/renderer/extensions/c...
File chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc (left):

https://codereview.chromium.org/564913003/diff/1/chrome/renderer/extensions/c...
chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc:277: if
(context->GetAvailability("webViewInternal").is_available()) {
On 2014/09/11 19:27:59, lfg wrote:
> On 2014/09/11 19:24:03, Fady Samuel wrote:
> > This check should be done in extensions as well.
> 
> I'll do it in a future patch. This patch only moves web_view.js to extensions,
> it still doesn't expose it in extensions (i.e.
module_system->Require("webView")
> still isn't called from extensions).
> 
> 

I'm surprised/confused that webview works at all then if we're not injecting
webView anywhere? Am I missing something?

[lfg, 2014-09-11 19:33:33]

On 2014/09/11 19:31:40, Fady Samuel wrote:
>
https://codereview.chromium.org/564913003/diff/1/chrome/renderer/extensions/c...
> File chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc
(left):
> 
>
https://codereview.chromium.org/564913003/diff/1/chrome/renderer/extensions/c...
> chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc:277: if
> (context->GetAvailability("webViewInternal").is_available()) {
> On 2014/09/11 19:27:59, lfg wrote:
> > On 2014/09/11 19:24:03, Fady Samuel wrote:
> > > This check should be done in extensions as well.
> > 
> > I'll do it in a future patch. This patch only moves web_view.js to
extensions,
> > it still doesn't expose it in extensions (i.e.
> module_system->Require("webView")
> > still isn't called from extensions).
> > 
> > 
> 
> I'm surprised/confused that webview works at all then if we're not injecting
> webView anywhere? Am I missing something?

We are requiring chromeWebView, and chromeWebView requires webView, so it gets
included indirectly.

[Fady Samuel, 2014-09-11 19:41:56]

On 2014/09/11 19:33:33, lfg wrote:
> On 2014/09/11 19:31:40, Fady Samuel wrote:
> >
>
https://codereview.chromium.org/564913003/diff/1/chrome/renderer/extensions/c...
> > File chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc
> (left):
> > 
> >
>
https://codereview.chromium.org/564913003/diff/1/chrome/renderer/extensions/c...
> > chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc:277: if
> > (context->GetAvailability("webViewInternal").is_available()) {
> > On 2014/09/11 19:27:59, lfg wrote:
> > > On 2014/09/11 19:24:03, Fady Samuel wrote:
> > > > This check should be done in extensions as well.
> > > 
> > > I'll do it in a future patch. This patch only moves web_view.js to
> extensions,
> > > it still doesn't expose it in extensions (i.e.
> > module_system->Require("webView")
> > > still isn't called from extensions).
> > > 
> > > 
> > 
> > I'm surprised/confused that webview works at all then if we're not injecting
> > webView anywhere? Am I missing something?
> 
> We are requiring chromeWebView, and chromeWebView requires webView, so it gets
> included indirectly.

Ohh OK lgtm

[lfg, 2014-09-11 19:46:39]

lfg@chromium.org changed reviewers:
+ rockot@chromium.org, sky@chromium.org

[lfg, 2014-09-11 19:46:40]

sky@chromium.org: Please review changes in 

*.grd files

rockot@chromium.org: Please review changes in 

*extensions*

[sky, 2014-09-11 19:57:36]

LGTM

[Ken Rockot(use gerrit already), 2014-09-11 20:01:25]

lgtm

[lfg, 2014-09-11 20:01:46]

The CQ bit was checked by lfg@chromium.org

[lazyboy, 2014-09-11 20:02:04]

https://codereview.chromium.org/564913003/diff/1/chrome/renderer/resources/ex...
File chrome/renderer/resources/extensions/chrome_web_view.js (right):

https://codereview.chromium.org/564913003/diff/1/chrome/renderer/resources/ex...
chrome/renderer/resources/extensions/chrome_web_view.js:7: var WebViewInternal =
require('webView').WebViewInternal
nit: Sort these.

https://codereview.chromium.org/564913003/diff/1/chrome/renderer/resources/ex...
chrome/renderer/resources/extensions/chrome_web_view.js:11: var CreateEvent =
function(name) {
Use the one defined in web_view_events.js
var CreateEvent = require('webViewEvents').CreateEvent;

https://codereview.chromium.org/564913003/diff/1/chrome/renderer/resources/ex...
chrome/renderer/resources/extensions/chrome_web_view.js:38: * This will be
overridden in web_view_experimental.js to implement contextmenu
s/web_view_experimental/chrome_web_view_experimental

https://codereview.chromium.org/564913003/diff/1/extensions/renderer/resource...
File extensions/renderer/resources/web_view.js (right):

https://codereview.chromium.org/564913003/diff/1/extensions/renderer/resource...
extensions/renderer/resources/web_view.js:17: var ChromeWebView =
require('chromeWebViewInternal').ChromeWebView
Accidental change? line #13 already includes this.

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

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

[lfg, 2014-09-11 20:09:21]

The CQ bit was unchecked by lfg@chromium.org

[lfg, 2014-09-11 21:17:20]

lazyboy@:

PTAL.

https://codereview.chromium.org/564913003/diff/1/chrome/renderer/resources/ex...
File chrome/renderer/resources/extensions/chrome_web_view.js (right):

https://codereview.chromium.org/564913003/diff/1/chrome/renderer/resources/ex...
chrome/renderer/resources/extensions/chrome_web_view.js:7: var WebViewInternal =
require('webView').WebViewInternal
On 2014/09/11 20:02:04, lazyboy wrote:
> nit: Sort these.

Done.

https://codereview.chromium.org/564913003/diff/1/chrome/renderer/resources/ex...
chrome/renderer/resources/extensions/chrome_web_view.js:11: var CreateEvent =
function(name) {
On 2014/09/11 20:02:04, lazyboy wrote:
> Use the one defined in web_view_events.js
> var CreateEvent = require('webViewEvents').CreateEvent;

Done.

https://codereview.chromium.org/564913003/diff/1/chrome/renderer/resources/ex...
chrome/renderer/resources/extensions/chrome_web_view.js:38: * This will be
overridden in web_view_experimental.js to implement contextmenu
On 2014/09/11 20:02:04, lazyboy wrote:
> s/web_view_experimental/chrome_web_view_experimental

Done.

https://codereview.chromium.org/564913003/diff/1/extensions/renderer/resource...
File extensions/renderer/resources/web_view.js (right):

https://codereview.chromium.org/564913003/diff/1/extensions/renderer/resource...
extensions/renderer/resources/web_view.js:17: var ChromeWebView =
require('chromeWebViewInternal').ChromeWebView
On 2014/09/11 20:02:04, lazyboy wrote:
> Accidental change? line #13 already includes this.

Yes -- and they are not needed anymore, since contextMenu moved to
chrome_web_view.

[lazyboy, 2014-09-11 21:23:27]

*web_view*.js LGTM with one nit.

https://codereview.chromium.org/564913003/diff/20001/chrome/renderer/resource...
File chrome/renderer/resources/extensions/chrome_web_view.js (right):

https://codereview.chromium.org/564913003/diff/20001/chrome/renderer/resource...
chrome/renderer/resources/extensions/chrome_web_view.js:11: var CreateEvent =
require('webViewEvents').CreateEvent;
nit: This one got unsorted :)

[lfg, 2014-09-11 21:28:27]

https://codereview.chromium.org/564913003/diff/20001/chrome/renderer/resource...
File chrome/renderer/resources/extensions/chrome_web_view.js (right):

https://codereview.chromium.org/564913003/diff/20001/chrome/renderer/resource...
chrome/renderer/resources/extensions/chrome_web_view.js:11: var CreateEvent =
require('webViewEvents').CreateEvent;
On 2014/09/11 21:23:26, lazyboy wrote:
> nit: This one got unsorted :)

Done.

[lfg, 2014-09-11 21:28:35]

The CQ bit was checked by lfg@chromium.org

[commit-bot: I haz the power, 2014-09-11 21:30:57]

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

[commit-bot: I haz the power, 2014-09-11 23:42:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-11 23:42:33]

Try jobs failed on following builders:
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[lfg, 2014-09-12 00:15:06]

The CQ bit was checked by lfg@chromium.org

[commit-bot: I haz the power, 2014-09-12 00:45:20]

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

[commit-bot: I haz the power, 2014-09-12 01:55:31]

Committed patchset #3 (id:40001) as 89e90e241a4fb2b567a60d9ddcdf2a9b3ee4f685

[commit-bot: I haz the power, 2014-09-12 02:00:55]

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