Extensions: Move targets in extensions/browser/guest_view to extensions/browser:browser_sources.

Extensions: Move targets in extensions/browser/guest_view to extensions/browser:browser_sources.

Currently the dependency chain is as follows: extensions/browser ->
extensions/browser/api -> extensions/browser:browser_sources ->
extensions/browser/guest_view. However extensions/browser/guest_view also has
cyclic dependencies on extensions/browser/api and
extensions/browser:browser_sources. This can be problematic since
extensions/browser/guest_view won't get any configs it needs for its dependency
on extensions/browser:browser_sources and extensions/browser/api. 

This CL moves the files in extensions/browser/guest_view, one layer up to
extensions/browser:browser_sources, hence removing the cyclic dependency between
extensions/browser/guest_view and extensions/browser:browser_sources. However, 
the cyclic dependency to extensions/browser/api is still not resolved.

BUG=730220
Review-Url: https://codereview.chromium.org/2932643004
Cr-Commit-Position: refs/heads/master@{#479231}
Committed: https://chromium.googlesource.com/chromium/src/+/05ba7bc22678febb0eff54435aa1acca3b06878b

[karandeepb, 2017-06-09 03:20:12]

The CQ bit was checked by karandeepb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-09 03:20:23]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-09 04:34:31]

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

[commit-bot: I haz the power, 2017-06-09 04:34:32]

Dry run: This issue passed the CQ dry run.

[karandeepb, 2017-06-09 23:25:32]

Description was changed from

==========
Extensions: Move targets in extensions/browser/guest_view to extensions/browser.
==========

to

==========
Extensions: Move targets in extensions/browser/guest_view to
extensions/browser:browser_sources.

Currently the dependency chain is as follows: extensions/browser ->
extensions/browser/api -> extensions/browser:browser_sources ->
extensions/browser/guest_view. However extensions/browser/guest_view also has
cyclic dependencies on extensions/browser/api and
extensions/browser:browser_sources. This can be problematic since
extensions/browser/guest_view won't get any configs it needs for its dependency
on extensions/browser:browser_sources and extensions/browser/api. 

This CL moves the files in extensions/browser/guest_view, one layer up to
extensions/browser:browser_sources, hence removing the cyclic dependency between
extensions/browser/guest_view and extensions/browser:browser_sources. However, 
the cyclic dependency to extensions/browser/api is still not resolved.

BUG=730220
==========

[karandeepb, 2017-06-09 23:33:15]

karandeepb@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[karandeepb, 2017-06-09 23:33:16]

PTAL Devlin. Was facing this issue while prototyping DNR. This does not solve
the problem completely, but I think is a step in the right direction.

[Devlin, 2017-06-12 15:20:20]

lgtm; thanks for cleaning this up.

+lazyboy@ for his thoughts on the webview comment.

In either case, this CL is fine, because it merely calls out existing dependency
issues; it itself does not add any.

https://codereview.chromium.org/2932643004/diff/1/extensions/browser/guest_vi...
File extensions/browser/guest_view/web_view/BUILD.gn (left):

https://codereview.chromium.org/2932643004/diff/1/extensions/browser/guest_vi...
extensions/browser/guest_view/web_view/BUILD.gn:5: source_set("web_view") {
This one's a bit disappointing.  Conceptually, I don't think webview *should*
depend on the extensions system (whereas other implementations, like
extension_options, clearly should).  I wonder if we can add a TODO to that
effect in the monolithic build file?

https://codereview.chromium.org/2932643004/diff/1/extensions/browser/guest_vi...
File extensions/browser/guest_view/web_view/web_ui/BUILD.gn (left):

https://codereview.chromium.org/2932643004/diff/1/extensions/browser/guest_vi...
extensions/browser/guest_view/web_view/web_ui/BUILD.gn:5: source_set("web_ui") {
This one can actually stay. :)  It has no dependencies on extension-y things.

[karandeepb, 2017-06-12 21:05:27]

Patchset #2 (id:20001) has been deleted

[karandeepb, 2017-06-12 21:10:49]

karandeepb@chromium.org changed reviewers:
+ brettw@chromium.org

[karandeepb, 2017-06-12 21:10:50]

+brettw@ for changes to
extensions/browser/guest_view/extension_view/whitelist/BUILD.gn.

Also, PTAL Devlin in case you have more comments.

https://codereview.chromium.org/2932643004/diff/1/extensions/browser/guest_vi...
File extensions/browser/guest_view/web_view/BUILD.gn (left):

https://codereview.chromium.org/2932643004/diff/1/extensions/browser/guest_vi...
extensions/browser/guest_view/web_view/BUILD.gn:5: source_set("web_view") {
On 2017/06/12 15:20:20, Devlin (sheriff Jun 9 - 12) wrote:
> This one's a bit disappointing.  Conceptually, I don't think webview *should*
> depend on the extensions system (whereas other implementations, like
> extension_options, clearly should).  I wonder if we can add a TODO to that
> effect in the monolithic build file?

Have added a TODO to that effect.

https://codereview.chromium.org/2932643004/diff/1/extensions/browser/guest_vi...
File extensions/browser/guest_view/web_view/web_ui/BUILD.gn (left):

https://codereview.chromium.org/2932643004/diff/1/extensions/browser/guest_vi...
extensions/browser/guest_view/web_view/web_ui/BUILD.gn:5: source_set("web_ui") {
On 2017/06/12 15:20:20, Devlin (sheriff Jun 9 - 12) wrote:
> This one can actually stay. :)  It has no dependencies on extension-y things.

Done.

[Devlin, 2017-06-13 22:03:06]

On 2017/06/12 21:10:50, karandeepb wrote:
> Also, PTAL Devlin in case you have more comments.

slgtm

[brettw, 2017-06-13 22:58:47]

lgtm

[karandeepb, 2017-06-13 23:03:53]

The CQ bit was checked by karandeepb@chromium.org

[karandeepb, 2017-06-13 23:03:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org
Link to the patchset: https://codereview.chromium.org/2932643004/#ps60001
(title: "Address review")

[commit-bot: I haz the power, 2017-06-13 23:04:47]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-14 00:49:47]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1497395033294800,
"parent_rev": "adeae69afea91b503de336275bad316a43080ee4", "commit_rev":
"05ba7bc22678febb0eff54435aa1acca3b06878b"}

[commit-bot: I haz the power, 2017-06-14 00:50:03]

Description was changed from

==========
Extensions: Move targets in extensions/browser/guest_view to
extensions/browser:browser_sources.

Currently the dependency chain is as follows: extensions/browser ->
extensions/browser/api -> extensions/browser:browser_sources ->
extensions/browser/guest_view. However extensions/browser/guest_view also has
cyclic dependencies on extensions/browser/api and
extensions/browser:browser_sources. This can be problematic since
extensions/browser/guest_view won't get any configs it needs for its dependency
on extensions/browser:browser_sources and extensions/browser/api. 

This CL moves the files in extensions/browser/guest_view, one layer up to
extensions/browser:browser_sources, hence removing the cyclic dependency between
extensions/browser/guest_view and extensions/browser:browser_sources. However, 
the cyclic dependency to extensions/browser/api is still not resolved.

BUG=730220
==========

to

==========
Extensions: Move targets in extensions/browser/guest_view to
extensions/browser:browser_sources.

Currently the dependency chain is as follows: extensions/browser ->
extensions/browser/api -> extensions/browser:browser_sources ->
extensions/browser/guest_view. However extensions/browser/guest_view also has
cyclic dependencies on extensions/browser/api and
extensions/browser:browser_sources. This can be problematic since
extensions/browser/guest_view won't get any configs it needs for its dependency
on extensions/browser:browser_sources and extensions/browser/api. 

This CL moves the files in extensions/browser/guest_view, one layer up to
extensions/browser:browser_sources, hence removing the cyclic dependency between
extensions/browser/guest_view and extensions/browser:browser_sources. However, 
the cyclic dependency to extensions/browser/api is still not resolved.

BUG=730220

Review-Url: https://codereview.chromium.org/2932643004
Cr-Commit-Position: refs/heads/master@{#479231}
Committed:
https://chromium.googlesource.com/chromium/src/+/05ba7bc22678febb0eff54435aa1...
==========

[commit-bot: I haz the power, 2017-06-14 00:50:04]

Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/05ba7bc22678febb0eff54435aa1...

[michaelpg, 2017-06-14 04:52:34]

Nice, thank you!