Clean up cloud printing aura stub code.

Clean up cloud printing aura stub code.


BUG=102144
TEST=NONE


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=113072

[Albert Bodenhamer, 2011-12-01 00:47:23]

http://codereview.chromium.org/8763007/diff/1/printing/printed_document.cc
File printing/printed_document.cc (right):

http://codereview.chromium.org/8763007/diff/1/printing/printed_document.cc#ne...
printing/printed_document.cc:237: NOTIMPLEMENTED();
I don't think NOTIMPLEMENTED is correct here.  NOTIMPLEMENTED will log an error.
 Doing nothing for this function is the CORRECT thing to do.  See logging.h line
913 for a discussion of NOTIMPLEMENTED

http://codereview.chromium.org/8763007/diff/1/printing/printed_document_gtk.cc
File printing/printed_document_gtk.cc (right):

http://codereview.chromium.org/8763007/diff/1/printing/printed_document_gtk.c...
printing/printed_document_gtk.cc:26: #if !defined(OS_CHROMEOS)
Remove this #if.  This whole file should only compile in on non-chromeos builds.

[Albert Bodenhamer, 2011-12-01 01:09:05]

http://codereview.chromium.org/8763007/diff/1/printing/printed_document.cc
File printing/printed_document.cc (right):

http://codereview.chromium.org/8763007/diff/1/printing/printed_document.cc#ne...
printing/printed_document.cc:233: #if defined(OS_POSIX) && defined(USE_AURA)
Shouldn't this be OS_CHROMEOS || USE_AURA?

http://codereview.chromium.org/8763007/diff/1/printing/printing.gyp
File printing/printing.gyp (left):

http://codereview.chromium.org/8763007/diff/1/printing/printing.gyp#oldcode39
printing/printing.gyp:39: 'image_cairo.cc',
Also remove image_cairo

http://codereview.chromium.org/8763007/diff/1/printing/printing.gyp
File printing/printing.gyp (right):

http://codereview.chromium.org/8763007/diff/1/printing/printing.gyp#newcode58
printing/printing.gyp:58: 'printed_document_gtk.cc',
If you've added the function to printed_document.cc you no longer need this
file.

[jennyz, 2011-12-02 14:40:17]

http://codereview.chromium.org/8763007/diff/1/printing/printed_document.cc
File printing/printed_document.cc (right):

http://codereview.chromium.org/8763007/diff/1/printing/printed_document.cc#ne...
printing/printed_document.cc:233: #if defined(OS_POSIX) && defined(USE_AURA)
On 2011/12/01 01:09:05, Albert Bodenhamer wrote:
> Shouldn't this be OS_CHROMEOS || USE_AURA?
This function signature is only declared for OS_POSIX in printed_document.h, so
defined(OS_POSIX) is needed, are you sure you want this for OS_CHROMEOS non-aura
build? I thought  that will use gtk implementation.

http://codereview.chromium.org/8763007/diff/1/printing/printed_document.cc#ne...
printing/printed_document.cc:237: NOTIMPLEMENTED();
On 2011/12/01 00:47:23, Albert Bodenhamer wrote:
> I don't think NOTIMPLEMENTED is correct here.  NOTIMPLEMENTED will log an
error.
>  Doing nothing for this function is the CORRECT thing to do.  See logging.h
line
> 913 for a discussion of NOTIMPLEMENTED

Done.

http://codereview.chromium.org/8763007/diff/1/printing/printing.gyp
File printing/printing.gyp (left):

http://codereview.chromium.org/8763007/diff/1/printing/printing.gyp#oldcode39
printing/printing.gyp:39: 'image_cairo.cc',
On 2011/12/01 01:09:05, Albert Bodenhamer wrote:
> Also remove image_cairo

Done. Renamed it to be image_linux.cc.

http://codereview.chromium.org/8763007/diff/1/printing/printing.gyp
File printing/printing.gyp (right):

http://codereview.chromium.org/8763007/diff/1/printing/printing.gyp#newcode58
printing/printing.gyp:58: 'printed_document_gtk.cc',
On 2011/12/01 01:09:05, Albert Bodenhamer wrote:
> If you've added the function to printed_document.cc you no longer need this
> file.
The gtk implementation for no non-aura builds, and the one in printed_docuemt.cc
is the default no-op stub for aura.

[Albert Bodenhamer, 2011-12-02 17:57:16]

http://codereview.chromium.org/8763007/diff/1/printing/printed_document.cc
File printing/printed_document.cc (right):

http://codereview.chromium.org/8763007/diff/1/printing/printed_document.cc#ne...
printing/printed_document.cc:233: #if defined(OS_POSIX) && defined(USE_AURA)
On chrome os there is no native printing, only cloud print, so no need to use
the GTK version.

You can do OS_POSIX || USE_AURA, but I'm afraid you'll get a symbol collision on
Linux since it needs to pull in the GTK version.  You don't want && since you'll
want this for the non-aura chromeos build.

On 2011/12/02 14:40:18, jennyz wrote:
> On 2011/12/01 01:09:05, Albert Bodenhamer wrote:
> > Shouldn't this be OS_CHROMEOS || USE_AURA?
> This function signature is only declared for OS_POSIX in printed_document.h,
so
> defined(OS_POSIX) is needed, are you sure you want this for OS_CHROMEOS
non-aura
> build? I thought  that will use gtk implementation.

http://codereview.chromium.org/8763007/diff/1/printing/printing.gyp
File printing/printing.gyp (left):

http://codereview.chromium.org/8763007/diff/1/printing/printing.gyp#oldcode39
printing/printing.gyp:39: 'image_cairo.cc',
Can't you just remove it completely?

On 2011/12/02 14:40:18, jennyz wrote:
> On 2011/12/01 01:09:05, Albert Bodenhamer wrote:
> > Also remove image_cairo
> 
> Done. Renamed it to be image_linux.cc.

http://codereview.chromium.org/8763007/diff/1/printing/printing.gyp
File printing/printing.gyp (right):

http://codereview.chromium.org/8763007/diff/1/printing/printing.gyp#newcode58
printing/printing.gyp:58: 'printed_document_gtk.cc',
OK
On 2011/12/02 14:40:18, jennyz wrote:
> On 2011/12/01 01:09:05, Albert Bodenhamer wrote:
> > If you've added the function to printed_document.cc you no longer need this
> > file.
> The gtk implementation for no non-aura builds, and the one in
printed_docuemt.cc
> is the default no-op stub for aura.

[Albert Bodenhamer, 2011-12-02 22:29:23]

lgtm

[commit-bot: I haz the power, 2011-12-05 22:21:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jennyz@chromium.org/8763007/7001

[commit-bot: I haz the power, 2011-12-06 00:23:45]

Change committed as 113072