Moving files from //chrome to //components/printing

Moving files from //chrome to //components/printing

Other changes:

- Modifying #includes and .gypi to target the new paths
- Modifying DEPS files to allow the new paths
- New gn, gypi, DEPS and OWNERS files for the new //components/printing

BUG=444883
Committed: https://crrev.com/445b5974445aa9a0f8afc87214f9dff090d5255c
Cr-Commit-Position: refs/heads/master@{#311293}

[dgn, 2014-12-29 17:34:10]

dgn@chromium.org changed reviewers:
+ vitalybuka@chromium.org

[dgn, 2014-12-29 17:34:11]

PTAL

In this patch I only moved the files and updated the build files. (no new build
files, etc.)

Lots of files changed here, but it's mostly #include updates.
I'll try to make the next patches smaller since I will be able to make more
localized changes.

[Vitaly Buka (NO REVIEWS), 2015-01-05 05:55:57]

On 2014/12/29 17:34:11, dgn wrote:
> PTAL
> 
> In this patch I only moved the files and updated the build files. (no new
build
> files, etc.)
> 
> Lots of files changed here, but it's mostly #include updates.
> I'll try to make the next patches smaller since I will be able to make more
> localized changes.

1. we can't just change paths in gyp files, please create build files.
2. please move only renderer in this CL, as it was in the prev CL. we don't need
to move browser yet.

[Vitaly Buka (NO REVIEWS), 2015-01-05 17:26:07]

thanks, almost there

https://codereview.chromium.org/811563008/diff/100001/components/printing/ren...
File components/printing/renderer/BUILD.gn (right):

https://codereview.chromium.org/811563008/diff/100001/components/printing/ren...
components/printing/renderer/BUILD.gn:5: static_library("renderer") {
printing_renderer

https://codereview.chromium.org/811563008/diff/100001/components/printing/ren...
components/printing/renderer/BUILD.gn:21: "//chrome/common",
components MUST NOT depend on chrome
chrome can depend on component
you need to move messages and some switches

[dgn, 2015-01-05 18:52:52]

Sorry, I didn't completely understand your comments on the previous CL. I
thought I should make smaller CLs, first moving the files, then removing the
dependencies, then adding proper build scripts.

If I have to avoid at all times dependencies to //chrome, then I have to add the
delegates[1] first before moving the files out of //chrome.

On windows x64, the old code raises warnings considered as errors[2]. Should I
attempt to fix it, suppress the warnings (how can I do that? I guess that's how
it was in the old file) or ignore the failure for now and fix it in another
patch?

[1]: https://codereview.chromium.org/791133006/
[2]:
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...

https://codereview.chromium.org/811563008/diff/100001/components/printing/ren...
File components/printing/renderer/BUILD.gn (right):

https://codereview.chromium.org/811563008/diff/100001/components/printing/ren...
components/printing/renderer/BUILD.gn:5: static_library("renderer") {
On 2015/01/05 17:26:07, Vitaly Buka wrote:
> printing_renderer

Done.

[Vitaly Buka (NO REVIEWS), 2015-01-08 00:21:16]

https://codereview.chromium.org/811563008/diff/140001/chrome/renderer/chrome_...
File chrome/renderer/chrome_content_renderer_client.cc (right):

https://codereview.chromium.org/811563008/diff/140001/chrome/renderer/chrome_...
chrome/renderer/chrome_content_renderer_client.cc:131: // TODO(dgn)
?

[Vitaly Buka (NO REVIEWS), 2015-01-08 22:46:16]

lgtm

https://codereview.chromium.org/811563008/diff/180001/components/printing/ren...
File components/printing/renderer/BUILD.gn (right):

https://codereview.chromium.org/811563008/diff/180001/components/printing/ren...
components/printing/renderer/BUILD.gn:28: cflags = [ "/wd4267" ]  # TODO(dgn):
crbug.com/167187 fix size_t to int truncations.
long line
put comment on line before csflag =

[dgn, 2015-01-09 10:47:18]

dgn@chromium.org changed reviewers:
+ blundell@chromium.org, dcheng@chromium.org, thestig@chromium.org

[dgn, 2015-01-09 10:47:20]

blundell@chromium.org: Please review changes in /components. This patch creates
/components/printing

dcheng@chromium.org: Please review changes in print_messages.h. I moved it to
/components/printing

thestig@chromium.org: Please review changes in /chrome. This patch moves some
files so their paths have to be update in #includes and build files.

https://codereview.chromium.org/811563008/diff/140001/chrome/renderer/chrome_...
File chrome/renderer/chrome_content_renderer_client.cc (right):

https://codereview.chromium.org/811563008/diff/140001/chrome/renderer/chrome_...
chrome/renderer/chrome_content_renderer_client.cc:131: // TODO(dgn)
On 2015/01/08 00:21:16, Vitaly Buka wrote:
> ?

Removed

https://codereview.chromium.org/811563008/diff/180001/components/printing/ren...
File components/printing/renderer/BUILD.gn (right):

https://codereview.chromium.org/811563008/diff/180001/components/printing/ren...
components/printing/renderer/BUILD.gn:28: cflags = [ "/wd4267" ]  # TODO(dgn):
crbug.com/167187 fix size_t to int truncations.
On 2015/01/08 22:46:16, Vitaly Buka wrote:
> long line
> put comment on line before csflag = 

Done.

[blundell, 2015-01-09 12:20:02]

//components lgtm

[Lei Zhang, 2015-01-09 21:55:17]

Looking outside of chrome:

- You need security OWNERS for IPC changes to print_message.*, with set
noparent.

- It would be helpful to have components/printing/README to explain the
relationship between printing/, components/printing/, and higher level up code.
See components/*/README for other examples.

https://codereview.chromium.org/811563008/diff/200001/components/printing/ren...
File components/printing/renderer/DEPS (right):

https://codereview.chromium.org/811563008/diff/200001/components/printing/ren...
components/printing/renderer/DEPS:2: "+net",
You can be more specific and just go with net/base

https://codereview.chromium.org/811563008/diff/200001/components/printing/ren...
components/printing/renderer/DEPS:2: "+net",
alphabetical order please, same in other DEPS files.

[Lei Zhang, 2015-01-09 22:02:42]

https://codereview.chromium.org/811563008/diff/200001/chrome/DEPS
File chrome/DEPS (right):

https://codereview.chromium.org/811563008/diff/200001/chrome/DEPS#newcode21
chrome/DEPS:21: "+components/printing",
Some of the other components rules in this file are slightly loose and do not
set a good example. Here, you should only allow components/printing/common, and
chrome/renderer/DEPS should allow components/printing/renderer.

I'll fix the other ones separately.

https://codereview.chromium.org/811563008/diff/200001/chrome/browser/browser_...
File chrome/browser/browser_resources.grd (left):

https://codereview.chromium.org/811563008/diff/200001/chrome/browser/browser_...
chrome/browser/browser_resources.grd:258: <include name="IDR_PRINT_PREVIEW_PAGE"
file="resources\print_preview\print_preview_page.html" flattenhtml="true"
allowexternalscript="false" type="BINDATA" />
Why is only this resource file being moved?

https://codereview.chromium.org/811563008/diff/200001/chrome/renderer/BUILD.gn
File chrome/renderer/BUILD.gn (right):

https://codereview.chromium.org/811563008/diff/200001/chrome/renderer/BUILD.g...
chrome/renderer/BUILD.gn:164: # TODO(GYP)
This entire block should be moved out of this file and into
components/printing/renderer/BUILD.gn

[Vitaly Buka (NO REVIEWS), 2015-01-09 23:29:48]

https://codereview.chromium.org/811563008/diff/200001/chrome/browser/browser_...
File chrome/browser/browser_resources.grd (left):

https://codereview.chromium.org/811563008/diff/200001/chrome/browser/browser_...
chrome/browser/browser_resources.grd:258: <include name="IDR_PRINT_PREVIEW_PAGE"
file="resources\print_preview\print_preview_page.html" flattenhtml="true"
allowexternalscript="false" type="BINDATA" />
On 2015/01/09 22:02:42, Lei Zhang wrote:
> Why is only this resource file being moved?

this change moves only renderer part of printing.
renderer uses this template for header/footer printing
other resources are used on browser side

[dgn, 2015-01-12 20:22:39]

https://codereview.chromium.org/811563008/diff/200001/chrome/DEPS
File chrome/DEPS (right):

https://codereview.chromium.org/811563008/diff/200001/chrome/DEPS#newcode21
chrome/DEPS:21: "+components/printing",
On 2015/01/09 22:02:42, Lei Zhang wrote:
> Some of the other components rules in this file are slightly loose and do not
> set a good example. Here, you should only allow components/printing/common,
and
> chrome/renderer/DEPS should allow components/printing/renderer.
> 
> I'll fix the other ones separately.

I restored this file and just added components/printing/common in browser and
renderer. Is that better?

https://codereview.chromium.org/811563008/diff/200001/chrome/renderer/BUILD.gn
File chrome/renderer/BUILD.gn (right):

https://codereview.chromium.org/811563008/diff/200001/chrome/renderer/BUILD.g...
chrome/renderer/BUILD.gn:164: # TODO(GYP)
On 2015/01/09 22:02:42, Lei Zhang wrote:
> This entire block should be moved out of this file and into
> components/printing/renderer/BUILD.gn

I guess I could fix the TODO as well while I'm at it. But I'm not sure to get
it. What is supposed to be done? The file is included in the gypi and BUILD.gn
files already.

https://codereview.chromium.org/811563008/diff/200001/components/printing/ren...
File components/printing/renderer/DEPS (right):

https://codereview.chromium.org/811563008/diff/200001/components/printing/ren...
components/printing/renderer/DEPS:2: "+net",
On 2015/01/09 21:55:17, Lei Zhang wrote:
> You can be more specific and just go with net/base

Done.

https://codereview.chromium.org/811563008/diff/200001/components/printing/ren...
components/printing/renderer/DEPS:2: "+net",
On 2015/01/09 21:55:16, Lei Zhang wrote:
> alphabetical order please, same in other DEPS files.

Done.

[Lei Zhang, 2015-01-12 21:24:21]

lgtm

https://codereview.chromium.org/811563008/diff/200001/chrome/DEPS
File chrome/DEPS (right):

https://codereview.chromium.org/811563008/diff/200001/chrome/DEPS#newcode21
chrome/DEPS:21: "+components/printing",
On 2015/01/12 20:22:39, dgn wrote:
> On 2015/01/09 22:02:42, Lei Zhang wrote:
> > Some of the other components rules in this file are slightly loose and do
not
> > set a good example. Here, you should only allow components/printing/common,
> and
> > chrome/renderer/DEPS should allow components/printing/renderer.
> > 
> > I'll fix the other ones separately.
> 
> I restored this file and just added components/printing/common in browser and
> renderer. Is that better?

What I meant was:

- Put +components/printing/common here.
- Put +components/printing/renderer in chrome/renderer/DEPS.

https://codereview.chromium.org/811563008/diff/200001/chrome/renderer/BUILD.gn
File chrome/renderer/BUILD.gn (right):

https://codereview.chromium.org/811563008/diff/200001/chrome/renderer/BUILD.g...
chrome/renderer/BUILD.gn:164: # TODO(GYP)
On 2015/01/12 20:22:39, dgn wrote:
> On 2015/01/09 22:02:42, Lei Zhang wrote:
> > This entire block should be moved out of this file and into
> > components/printing/renderer/BUILD.gn
> 
> I guess I could fix the TODO as well while I'm at it. But I'm not sure to get
> it. What is supposed to be done? The file is included in the gypi and BUILD.gn
> files already.

I guess someone added the file to the GN build and forgot about this, so
deleting it is fine.

[dcheng, 2015-01-12 23:41:12]

rslgtm for IPC changes

I wonder if there's some way we can use the PRESUBMIT to enforce IPC owner
reviews, rather than OWNERS files. It seems like similar steps have accidentally
been forgotten for other things that were componentized. I'll file a bug for
this.

[dcheng, 2015-01-12 23:41:22]

lgtm, even

[dgn, 2015-01-13 12:22:42]

dgn@chromium.org changed reviewers:
+ darin@chromium.org

[dgn, 2015-01-13 12:22:44]

darin@chromium.org: Please review the dependencies in the new DEPS files:
components/printing/DEPS and components/printing/renderer/DEPS

The ones for which LGTM is missing are:

    '+content/public',
    '+ipc',
    '+net/base',
    '+skia/ext',
    '+ui/base',
    '+ui/gfx',

[darin (slow to review), 2015-01-13 17:45:10]

LGTM for components/../DEPS

[dgn, 2015-01-13 18:02:18]

The CQ bit was checked by dgn@chromium.org

[commit-bot: I haz the power, 2015-01-13 18:03:26]

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

[commit-bot: I haz the power, 2015-01-13 18:13:38]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2015-01-13 18:15:21]

Patchset 12 (id:??) landed as
https://crrev.com/445b5974445aa9a0f8afc87214f9dff090d5255c
Cr-Commit-Position: refs/heads/master@{#311293}