Refine //extensions DEPS on //ui

Refine //extensions DEPS on //ui

This patch makes more explicit the set of dependencies
allowed from //extensions onto //ui.

//extensions was allowing dependencies on all of //ui
by default. This is unfortunate for people who want to
introduce new code into //extensions only to be shot
down because they depend on pieces of ui that aren't
allowed.

BUG=None

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

[Ken Rockot(use gerrit already), 2014-11-03 17:44:12]

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

[Ken Rockot(use gerrit already), 2014-11-03 17:44:13]

James, PTAL. The question of what ui is really allowed in //extensions has come
up on several occasions from people looking to add new API code.

[James Cook, 2014-11-03 21:35:20]

LGTM with nits. However.... I feel we should merge extensions/DEPS,
extensions/browser/DEPS and extensions/common/DEPS, or at least move the vast
majority of the DEPS rules to the parent. The small additional risk of people
including browser code in the renderer or vice versa is heavily outweighed by
giving people a single place to look to understand the dependency tree.  That
doesn't have to happen in this CL but I think it needs to happen.

https://codereview.chromium.org/702513002/diff/1/extensions/DEPS
File extensions/DEPS (right):

https://codereview.chromium.org/702513002/diff/1/extensions/DEPS#newcode16
extensions/DEPS:16: "-ui",
Add a comment with the rationale for excluding all of UI -- maybe just provide
an example of something you are trying to disallow.

https://codereview.chromium.org/702513002/diff/1/extensions/shell/browser/DEPS
File extensions/shell/browser/DEPS (right):

https://codereview.chromium.org/702513002/diff/1/extensions/shell/browser/DEP...
extensions/shell/browser/DEPS:31: "+ui/aura",
Likewise I would comment here that we don't use any particular UI toolkit, just
pure Aura.

[Ken Rockot(use gerrit already), 2014-11-03 22:05:58]

I feel pretty strongly about keeping dependencies separated among the
second-level paths like browser and renderer.

For me it's not as much about risk as it is about clear communication and
correctness. DEPS should be an authoritative source of information regarding
what dependencies are allowed.

Adding "+v8" to top-level //extensions/DEPS means one of two things:

1. We have to add "-v8" to //extensions/browser/DEPS, //extensions/common/DEPS,
and //extensions/utility/DEPS (coming soon)

OR

2. We concede that our DEPS will not act as an authoritative source of
information regarding what dependencies are allowed.


#2 seems to betray of the central purpose of DEPS, and I think #1 is noisier and
more confusing than our current situation.

[James Cook, 2014-11-03 22:46:10]

On 2014/11/03 22:05:58, Ken Rockot wrote:
> I feel pretty strongly about keeping dependencies separated among the
> second-level paths like browser and renderer.
> 
> For me it's not as much about risk as it is about clear communication and
> correctness. DEPS should be an authoritative source of information regarding
> what dependencies are allowed.
> 
> Adding "+v8" to top-level //extensions/DEPS means one of two things:
> 
> 1. We have to add "-v8" to //extensions/browser/DEPS,
//extensions/common/DEPS,
> and //extensions/utility/DEPS (coming soon)
> 
> OR
> 
> 2. We concede that our DEPS will not act as an authoritative source of
> information regarding what dependencies are allowed.
> 
> 
> #2 seems to betray of the central purpose of DEPS, and I think #1 is noisier
and
> more confusing than our current situation.

Still LGTM

Based on offline discussion I will look into our overall DEPS structure as a
low-priority task.

[Ken Rockot(use gerrit already), 2014-11-04 16:50:23]

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

[Ken Rockot(use gerrit already), 2014-11-04 16:50:24]

+thakis@ for ui dependency shuffling

https://codereview.chromium.org/702513002/diff/1/extensions/DEPS
File extensions/DEPS (right):

https://codereview.chromium.org/702513002/diff/1/extensions/DEPS#newcode16
extensions/DEPS:16: "-ui",
On 2014/11/03 21:35:20, James Cook wrote:
> Add a comment with the rationale for excluding all of UI -- maybe just provide
> an example of something you are trying to disallow.

Done.

https://codereview.chromium.org/702513002/diff/1/extensions/shell/browser/DEPS
File extensions/shell/browser/DEPS (right):

https://codereview.chromium.org/702513002/diff/1/extensions/shell/browser/DEP...
extensions/shell/browser/DEPS:31: "+ui/aura",
On 2014/11/03 21:35:20, James Cook wrote:
> Likewise I would comment here that we don't use any particular UI toolkit,
just
> pure Aura.

Done.

[Nico, 2014-11-04 17:04:11]

ui lgtm

[Ken Rockot(use gerrit already), 2014-11-04 17:04:43]

The CQ bit was checked by rockot@chromium.org

[commit-bot: I haz the power, 2014-11-04 17:05:27]

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

[commit-bot: I haz the power, 2014-11-04 17:23:43]

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

[commit-bot: I haz the power, 2014-11-04 17:23:44]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_32 on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ken Rockot(use gerrit already), 2014-11-04 18:26:28]

The CQ bit was checked by rockot@chromium.org

[commit-bot: I haz the power, 2014-11-04 18:27:56]

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

[commit-bot: I haz the power, 2014-11-04 19:09:39]

Committed patchset #3 (id:30001)

[commit-bot: I haz the power, 2014-11-04 19:10:16]

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