Modify chrome to accept a pdf file on the command line and upload it to Cloud...

Modify chrome to accept a pdf file on the command line and upload it to Cloud Print via the Cloud Print dialog
Contributed by abodenha@google.com
BUG=none
TEST=Run Chrome on Windows with --cloud-print-file=<Path to PDF> and --cloud-print-job-title

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

[sanjeevr, 2011-01-11 21:39:55]

Added Scott as a reviewer since he wrote the Cloud Print dialog code.

http://codereview.chromium.org/6038008/diff/26002/chrome/browser/browser_main.cc
File chrome/browser/browser_main.cc (right):

http://codereview.chromium.org/6038008/diff/26002/chrome/browser/browser_main...
chrome/browser/browser_main.cc:1630:
parsed_command_line.GetSwitchValuePath(switches::kCloudPrintFile);
If GetSwitchValuePath returns empty, perhaps we should pretend like this switch
was not specified?

http://codereview.chromium.org/6038008/diff/26002/chrome/browser/printing/pri...
File chrome/browser/printing/print_dialog_cloud.cc (right):

http://codereview.chromium.org/6038008/diff/26002/chrome/browser/printing/pri...
chrome/browser/printing/print_dialog_cloud.cc:435: // Keep the browser alive
while we are showing the notification.
Nit: Change comment, this is not a notification. Also elaborate on why this is
needed.

http://codereview.chromium.org/6038008/diff/26002/chrome/browser/printing/pri...
chrome/browser/printing/print_dialog_cloud.cc:483: // Keep the browser alive
while we are showing the notification.
Same here.

http://codereview.chromium.org/6038008/diff/26002/chrome/browser/printing/pri...
chrome/browser/printing/print_dialog_cloud.cc:544:
switches::kCloudPrintJobTitle);
We should probably refactor this and have the title as well as modal passed in
to this class.

http://codereview.chromium.org/6038008/diff/26002/chrome/common/chrome_switch...
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/6038008/diff/26002/chrome/common/chrome_switch...
chrome/common/chrome_switches.cc:116: const char kCloudPrintFile[]              
 = "cloud-print-file";
Nit: Wondering if we should perhaps use another name (though can't think of
anything better at this time :) )

[Scott Byer, 2011-01-11 21:46:31]

http://codereview.chromium.org/6038008/diff/26002/chrome/browser/printing/pri...
File chrome/browser/printing/print_dialog_cloud.cc (right):

http://codereview.chromium.org/6038008/diff/26002/chrome/browser/printing/pri...
chrome/browser/printing/print_dialog_cloud.cc:569: if (browser_) {
I'd be surprised if this pattern wasn't elsewhere in the code; it seems like it
wants re-factoring.

[abodenha, 2011-01-12 01:30:50]

I've updated code in response to your comments, but I'm running into test issues
on Mac and Linux trys.

I'll update again once the test issues are sorted, but please take another quick
look.

http://codereview.chromium.org/6038008/diff/26002/chrome/browser/browser_main.cc
File chrome/browser/browser_main.cc (right):

http://codereview.chromium.org/6038008/diff/26002/chrome/browser/browser_main...
chrome/browser/browser_main.cc:1630:
parsed_command_line.GetSwitchValuePath(switches::kCloudPrintFile);
On 2011/01/11 21:39:55, sanjeevr wrote:
> If GetSwitchValuePath returns empty, perhaps we should pretend like this
switch
> was not specified?

Done.

http://codereview.chromium.org/6038008/diff/26002/chrome/browser/printing/pri...
File chrome/browser/printing/print_dialog_cloud.cc (right):

http://codereview.chromium.org/6038008/diff/26002/chrome/browser/printing/pri...
chrome/browser/printing/print_dialog_cloud.cc:435: // Keep the browser alive
while we are showing the notification.
On 2011/01/11 21:39:55, sanjeevr wrote:
> Nit: Change comment, this is not a notification. Also elaborate on why this is
> needed.

Done.

http://codereview.chromium.org/6038008/diff/26002/chrome/browser/printing/pri...
chrome/browser/printing/print_dialog_cloud.cc:483: // Keep the browser alive
while we are showing the notification.
On 2011/01/11 21:39:55, sanjeevr wrote:
> Same here.

Done.

http://codereview.chromium.org/6038008/diff/26002/chrome/browser/printing/pri...
chrome/browser/printing/print_dialog_cloud.cc:544:
switches::kCloudPrintJobTitle);
Good call.  Refactoring makes the code much cleaner.  It also addresses Scott's
concern about the if(browser_) pattern.

On 2011/01/11 21:39:55, sanjeevr wrote:
> We should probably refactor this and have the title as well as modal passed in
> to this class.

http://codereview.chromium.org/6038008/diff/26002/chrome/browser/printing/pri...
chrome/browser/printing/print_dialog_cloud.cc:569: if (browser_) {
Done.

See comment about refactoring for modal and print_job_title

On 2011/01/11 21:46:31, Scott Byer wrote:
> I'd be surprised if this pattern wasn't elsewhere in the code; it seems like
it
> wants re-factoring.

http://codereview.chromium.org/6038008/diff/26002/chrome/common/chrome_switch...
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/6038008/diff/26002/chrome/common/chrome_switch...
chrome/common/chrome_switches.cc:116: const char kCloudPrintFile[]              
 = "cloud-print-file";
I'm not fond of the name either, but I'm having a hard time coming up with
something better.  Perhaps "cloud-print-upload-file" ?

On 2011/01/11 21:39:55, sanjeevr wrote:
> Nit: Wondering if we should perhaps use another name (though can't think of
> anything better at this time :) )

[Scott Byer, 2011-01-12 23:35:02]

http://codereview.chromium.org/6038008/diff/27003/chrome/browser/printing/pri...
File chrome/browser/printing/print_dialog_cloud.cc (right):

http://codereview.chromium.org/6038008/diff/27003/chrome/browser/printing/pri...
chrome/browser/printing/print_dialog_cloud.cc:544: if (modal_) {
When called to print a web page, the dialog should be modal (see comment in
render_message_filter_gtk.cc), but the title will need to come from the
TabContents in that case.

http://codereview.chromium.org/6038008/diff/27003/chrome/browser/renderer_hos...
File chrome/browser/renderer_host/render_message_filter_gtk.cc (right):

http://codereview.chromium.org/6038008/diff/27003/chrome/browser/renderer_hos...
chrome/browser/renderer_host/render_message_filter_gtk.cc:337:
PrintDialogCloud::CreatePrintDialogForPdf(it->second, string16(), false);
This changes behavior on ChromeOS - the dialog is currently modal (by necessity
until print preview is done).

[abodenha, 2011-01-13 00:00:44]

On Wed, Jan 12, 2011 at 3:35 PM, <scottbyer@chromium.org> wrote:

>
>
>
http://codereview.chromium.org/6038008/diff/27003/chrome/browser/printing/pri...
>
> File chrome/browser/printing/print_dialog_cloud.cc (right):
>
>
>
http://codereview.chromium.org/6038008/diff/27003/chrome/browser/printing/pri...
> chrome/browser/printing/print_dialog_cloud.cc:544: if (modal_) {
> When called to print a web page, the dialog should be modal (see comment
> in render_message_filter_gtk.cc), but the title will need to come from
> the TabContents in that case.
>
>
>
http://codereview.chromium.org/6038008/diff/27003/chrome/browser/renderer_hos...
> File chrome/browser/renderer_host/render_message_filter_gtk.cc (right):
>
>
>
http://codereview.chromium.org/6038008/diff/27003/chrome/browser/renderer_hos...
> chrome/browser/renderer_host/render_message_filter_gtk.cc:337:
> PrintDialogCloud::CreatePrintDialogForPdf(it->second, string16(),
> false);
> This changes behavior on ChromeOS - the dialog is currently modal (by
> necessity until print preview is done).
>
>
Perfect timing.  :-)  I was just uploading a new patch that addresses this.

I accidentally inverted the modality during my refactoring which is part of
what was causing my tries to fail.

There's a new patch up for review.  Tries are passing for win, mac, & linux.
 Please submit if you're happy with it.

Thanks.


>
> http://codereview.chromium.org/6038008/
>

[abodenha, 2011-01-13 00:05:55]

Correction.  I misread the try e-mail.  The win try is now failing.  I'll
update the thread when I have something ready.

On Wed, Jan 12, 2011 at 4:00 PM, Albert Bodenhamer <abodenha@google.com>wrote:

> On Wed, Jan 12, 2011 at 3:35 PM, <scottbyer@chromium.org> wrote:
>
>>
>>
>>
http://codereview.chromium.org/6038008/diff/27003/chrome/browser/printing/pri...
>>
>> File chrome/browser/printing/print_dialog_cloud.cc (right):
>>
>>
>>
http://codereview.chromium.org/6038008/diff/27003/chrome/browser/printing/pri...
>> chrome/browser/printing/print_dialog_cloud.cc:544: if (modal_) {
>> When called to print a web page, the dialog should be modal (see comment
>> in render_message_filter_gtk.cc), but the title will need to come from
>> the TabContents in that case.
>>
>>
>>
http://codereview.chromium.org/6038008/diff/27003/chrome/browser/renderer_hos...
>> File chrome/browser/renderer_host/render_message_filter_gtk.cc (right):
>>
>>
>>
http://codereview.chromium.org/6038008/diff/27003/chrome/browser/renderer_hos...
>> chrome/browser/renderer_host/render_message_filter_gtk.cc:337:
>> PrintDialogCloud::CreatePrintDialogForPdf(it->second, string16(),
>> false);
>> This changes behavior on ChromeOS - the dialog is currently modal (by
>> necessity until print preview is done).
>>
>>
> Perfect timing.  :-)  I was just uploading a new patch that addresses this.
>
> I accidentally inverted the modality during my refactoring which is part of
> what was causing my tries to fail.
>
> There's a new patch up for review.  Tries are passing for win, mac, &
> linux.  Please submit if you're happy with it.
>
> Thanks.
>
>
>>
>> http://codereview.chromium.org/6038008/
>>
>
>

[Scott Byer, 2011-01-13 00:08:22]

LGTM, waiting for Sanjeev's OK.

On 2011/01/13 00:05:55, abodenha wrote:
> Correction.  I misread the try e-mail.  The win try is now failing.  I'll
> update the thread when I have something ready.
> 
> On Wed, Jan 12, 2011 at 4:00 PM, Albert Bodenhamer <abodenha@google.com>wrote:
> 
> > On Wed, Jan 12, 2011 at 3:35 PM, <mailto:scottbyer@chromium.org> wrote:
> >
> >>
> >>
> >>
>
http://codereview.chromium.org/6038008/diff/27003/chrome/browser/printing/pri...
> >>
> >> File chrome/browser/printing/print_dialog_cloud.cc (right):
> >>
> >>
> >>
>
http://codereview.chromium.org/6038008/diff/27003/chrome/browser/printing/pri...
> >> chrome/browser/printing/print_dialog_cloud.cc:544: if (modal_) {
> >> When called to print a web page, the dialog should be modal (see comment
> >> in render_message_filter_gtk.cc), but the title will need to come from
> >> the TabContents in that case.
> >>
> >>
> >>
>
http://codereview.chromium.org/6038008/diff/27003/chrome/browser/renderer_hos...
> >> File chrome/browser/renderer_host/render_message_filter_gtk.cc (right):
> >>
> >>
> >>
>
http://codereview.chromium.org/6038008/diff/27003/chrome/browser/renderer_hos...
> >> chrome/browser/renderer_host/render_message_filter_gtk.cc:337:
> >> PrintDialogCloud::CreatePrintDialogForPdf(it->second, string16(),
> >> false);
> >> This changes behavior on ChromeOS - the dialog is currently modal (by
> >> necessity until print preview is done).
> >>
> >>
> > Perfect timing.  :-)  I was just uploading a new patch that addresses this.
> >
> > I accidentally inverted the modality during my refactoring which is part of
> > what was causing my tries to fail.
> >
> > There's a new patch up for review.  Tries are passing for win, mac, &
> > linux.  Please submit if you're happy with it.
> >
> > Thanks.
> >
> >
> >>
> >> http://codereview.chromium.org/6038008/
> >>
> >
> >

[Scott Byer, 2011-01-13 03:18:22]

Looks like trybot flake - just send the same patch again.

-Scott

On Wed, Jan 12, 2011 at 4:05 PM, Albert Bodenhamer <abodenha@google.com>wrote:

> Correction.  I misread the try e-mail.  The win try is now failing.  I'll
> update the thread when I have something ready.
>
>
> On Wed, Jan 12, 2011 at 4:00 PM, Albert Bodenhamer <abodenha@google.com>wrote:
>
>> On Wed, Jan 12, 2011 at 3:35 PM, <scottbyer@chromium.org> wrote:
>>
>>>
>>>
>>>
http://codereview.chromium.org/6038008/diff/27003/chrome/browser/printing/pri...
>>>
>>> File chrome/browser/printing/print_dialog_cloud.cc (right):
>>>
>>>
>>>
http://codereview.chromium.org/6038008/diff/27003/chrome/browser/printing/pri...
>>> chrome/browser/printing/print_dialog_cloud.cc:544: if (modal_) {
>>> When called to print a web page, the dialog should be modal (see comment
>>> in render_message_filter_gtk.cc), but the title will need to come from
>>> the TabContents in that case.
>>>
>>>
>>>
http://codereview.chromium.org/6038008/diff/27003/chrome/browser/renderer_hos...
>>> File chrome/browser/renderer_host/render_message_filter_gtk.cc (right):
>>>
>>>
>>>
http://codereview.chromium.org/6038008/diff/27003/chrome/browser/renderer_hos...
>>> chrome/browser/renderer_host/render_message_filter_gtk.cc:337:
>>> PrintDialogCloud::CreatePrintDialogForPdf(it->second, string16(),
>>> false);
>>> This changes behavior on ChromeOS - the dialog is currently modal (by
>>> necessity until print preview is done).
>>>
>>>
>> Perfect timing.  :-)  I was just uploading a new patch that addresses
>> this.
>>
>> I accidentally inverted the modality during my refactoring which is part
>> of what was causing my tries to fail.
>>
>> There's a new patch up for review.  Tries are passing for win, mac, &
>> linux.  Please submit if you're happy with it.
>>
>> Thanks.
>>
>>
>>>
>>> http://codereview.chromium.org/6038008/
>>>
>>
>>
>

[sanjeevr, 2011-01-13 18:03:36]

Mostly LGTM, I just have a couple of comments.

http://codereview.chromium.org/6038008/diff/82001/chrome/browser/browser_main.cc
File chrome/browser/browser_main.cc (right):

http://codereview.chromium.org/6038008/diff/82001/chrome/browser/browser_main...
chrome/browser/browser_main.cc:1633: #ifdef OS_WIN
Why is this part Windows-only?

http://codereview.chromium.org/6038008/diff/82001/chrome/browser/ui/browser_i...
File chrome/browser/ui/browser_init.cc (right):

http://codereview.chromium.org/6038008/diff/82001/chrome/browser/ui/browser_i...
chrome/browser/ui/browser_init.cc:1042: if
(command_line.HasSwitch(switches::kCloudPrintFile)) {
Here also we should check if there is a path actually specified.

[abodenha, 2011-01-13 19:03:59]

I've uploaded a new patch and started a new try

http://codereview.chromium.org/6038008/diff/82001/chrome/browser/browser_main.cc
File chrome/browser/browser_main.cc (right):

http://codereview.chromium.org/6038008/diff/82001/chrome/browser/browser_main...
chrome/browser/browser_main.cc:1633: #ifdef OS_WIN
On 2011/01/13 18:03:41, sanjeevr wrote:
> Why is this part Windows-only?
There are problems with the type conversion from CommandLine::StringType to
string16 on Linux and Mac.  It requires somewhat different handling on Linux and
Mac.  Since we're not implementing this on Linux or Mac yet...

I've reduced the scope of the #if slightly and added a comment.

I can try to create an equivalent code path for OS_POSIX but I don't have a
build enviroment set up for either at the moment so it's slow going.

http://codereview.chromium.org/6038008/diff/82001/chrome/browser/ui/browser_i...
File chrome/browser/ui/browser_init.cc (right):

http://codereview.chromium.org/6038008/diff/82001/chrome/browser/ui/browser_i...
chrome/browser/ui/browser_init.cc:1042: if
(command_line.HasSwitch(switches::kCloudPrintFile)) {
On 2011/01/13 18:03:41, sanjeevr wrote:
> Here also we should check if there is a path actually specified.

Done.

[sanjeevr, 2011-01-13 19:08:46]

LGTM

[abodenha, 2011-01-13 19:13:52]

Thanks.

Can one of you submit once the try completes?

On Thu, Jan 13, 2011 at 11:08 AM, <sanjeevr@chromium.org> wrote:

> LGTM
>
>
> http://codereview.chromium.org/6038008/
>

[Scott Byer, 2011-01-19 00:51:25]

Sorry, this slipped our minds.  I'll commit in the morning.

On 2011/01/13 19:13:52, abodenha wrote:
> Thanks.
> 
> Can one of you submit once the try completes?
> 
> On Thu, Jan 13, 2011 at 11:08 AM, <mailto:sanjeevr@chromium.org> wrote:
> 
> > LGTM
> >
> >
> > http://codereview.chromium.org/6038008/
> >

[abodenha, 2011-01-19 00:52:56]

Thanks!

On Tue, Jan 18, 2011 at 4:51 PM, <scottbyer@chromium.org> wrote:

> Sorry, this slipped our minds.  I'll commit in the morning.
>
>
> On 2011/01/13 19:13:52, abodenha wrote:
>
>> Thanks.
>>
>
>  Can one of you submit once the try completes?
>>
>
>  On Thu, Jan 13, 2011 at 11:08 AM, <mailto:sanjeevr@chromium.org> wrote:
>>
>
>  > LGTM
>> >
>> >
>> > http://codereview.chromium.org/6038008/
>> >
>>
>
>
>
> http://codereview.chromium.org/6038008/
>

[Scott Byer, 2011-01-19 18:23:14]

There was a conflict when merging up with trunk (one line).  It's easiest if you
go ahead and do that merge and upload the revised patch, and I'll get it
committed.  Thanks!

-Scott

On 2011/01/19 00:52:56, abodenha wrote:
> Thanks!
> 
> On Tue, Jan 18, 2011 at 4:51 PM, <mailto:scottbyer@chromium.org> wrote:
> 
> > Sorry, this slipped our minds.  I'll commit in the morning.
> >
> >
> > On 2011/01/13 19:13:52, abodenha wrote:
> >
> >> Thanks.
> >>
> >
> >  Can one of you submit once the try completes?
> >>
> >
> >  On Thu, Jan 13, 2011 at 11:08 AM, <mailto:sanjeevr@chromium.org> wrote:
> >>
> >
> >  > LGTM
> >> >
> >> >
> >> > http://codereview.chromium.org/6038008/
> >> >
> >>
> >
> >
> >
> > http://codereview.chromium.org/6038008/
> >

[abodenha, 2011-01-19 22:57:29]

Ah, the joys of waiting for re-sync/re-build. :-) I'll try to have a new
patch uploaded today.

Thanks.

On Wed, Jan 19, 2011 at 10:23 AM, <scottbyer@chromium.org> wrote:

>
> There was a conflict when merging up with trunk (one line).  It's easiest
> if you
> go ahead and do that merge and upload the revised patch, and I'll get it
> committed.  Thanks!
>
> -Scott
>
>
> On 2011/01/19 00:52:56, abodenha wrote:
>
>> Thanks!
>>
>
>  On Tue, Jan 18, 2011 at 4:51 PM, <mailto:scottbyer@chromium.org> wrote:
>>
>
>  > Sorry, this slipped our minds.  I'll commit in the morning.
>> >
>> >
>> > On 2011/01/13 19:13:52, abodenha wrote:
>> >
>> >> Thanks.
>> >>
>> >
>> >  Can one of you submit once the try completes?
>> >>
>> >
>> >  On Thu, Jan 13, 2011 at 11:08 AM, <mailto:sanjeevr@chromium.org>
>> wrote:
>> >>
>> >
>> >  > LGTM
>> >> >
>> >> >
>> >> > http://codereview.chromium.org/6038008/
>> >> >
>> >>
>> >
>> >
>> >
>> > http://codereview.chromium.org/6038008/
>> >
>>
>
>
>
> http://codereview.chromium.org/6038008/
>

[abodenha, 2011-01-20 01:37:35]

Conflicts are resolved, new patch uploaded, try started.

Unfortunately, I'm unable to build locally due to something that appears
unrelated (CEEE issue).

On Wed, Jan 19, 2011 at 2:57 PM, Albert Bodenhamer <abodenha@google.com>wrote:

> Ah, the joys of waiting for re-sync/re-build. :-) I'll try to have a new
> patch uploaded today.
>
> Thanks.
>
>
> On Wed, Jan 19, 2011 at 10:23 AM, <scottbyer@chromium.org> wrote:
>
>>
>> There was a conflict when merging up with trunk (one line).  It's easiest
>> if you
>> go ahead and do that merge and upload the revised patch, and I'll get it
>> committed.  Thanks!
>>
>> -Scott
>>
>>
>> On 2011/01/19 00:52:56, abodenha wrote:
>>
>>> Thanks!
>>>
>>
>>  On Tue, Jan 18, 2011 at 4:51 PM, <mailto:scottbyer@chromium.org> wrote:
>>>
>>
>>  > Sorry, this slipped our minds.  I'll commit in the morning.
>>> >
>>> >
>>> > On 2011/01/13 19:13:52, abodenha wrote:
>>> >
>>> >> Thanks.
>>> >>
>>> >
>>> >  Can one of you submit once the try completes?
>>> >>
>>> >
>>> >  On Thu, Jan 13, 2011 at 11:08 AM, <mailto:sanjeevr@chromium.org>
>>> wrote:
>>> >>
>>> >
>>> >  > LGTM
>>> >> >
>>> >> >
>>> >> > http://codereview.chromium.org/6038008/
>>> >> >
>>> >>
>>> >
>>> >
>>> >
>>> > http://codereview.chromium.org/6038008/
>>> >
>>>
>>
>>
>>
>> http://codereview.chromium.org/6038008/
>>
>
>

[abodenha, 2011-01-20 02:01:42]

Local issues are worked out.  Once the tries pass we should be good to go.

On Wed, Jan 19, 2011 at 5:30 PM, Albert Bodenhamer <abodenha@google.com>wrote:

> Conflicts are resolved, new patch uploaded, try started.
>
> Unfortunately, I'm unable to build locally due to something that appears
> unrelated (CEEE issue).
>
>
> On Wed, Jan 19, 2011 at 2:57 PM, Albert Bodenhamer <abodenha@google.com>wrote:
>
>> Ah, the joys of waiting for re-sync/re-build. :-) I'll try to have a new
>> patch uploaded today.
>>
>> Thanks.
>>
>>
>> On Wed, Jan 19, 2011 at 10:23 AM, <scottbyer@chromium.org> wrote:
>>
>>>
>>> There was a conflict when merging up with trunk (one line).  It's easiest
>>> if you
>>> go ahead and do that merge and upload the revised patch, and I'll get it
>>> committed.  Thanks!
>>>
>>> -Scott
>>>
>>>
>>> On 2011/01/19 00:52:56, abodenha wrote:
>>>
>>>> Thanks!
>>>>
>>>
>>>  On Tue, Jan 18, 2011 at 4:51 PM, <mailto:scottbyer@chromium.org> wrote:
>>>>
>>>
>>>  > Sorry, this slipped our minds.  I'll commit in the morning.
>>>> >
>>>> >
>>>> > On 2011/01/13 19:13:52, abodenha wrote:
>>>> >
>>>> >> Thanks.
>>>> >>
>>>> >
>>>> >  Can one of you submit once the try completes?
>>>> >>
>>>> >
>>>> >  On Thu, Jan 13, 2011 at 11:08 AM, <mailto:sanjeevr@chromium.org>
>>>> wrote:
>>>> >>
>>>> >
>>>> >  > LGTM
>>>> >> >
>>>> >> >
>>>> >> > http://codereview.chromium.org/6038008/
>>>> >> >
>>>> >>
>>>> >
>>>> >
>>>> >
>>>> > http://codereview.chromium.org/6038008/
>>>> >
>>>>
>>>
>>>
>>>
>>> http://codereview.chromium.org/6038008/
>>>
>>
>>
>

[Scott Byer, 2011-01-20 02:59:40]

I'll catch it first thing in the morning - I don't like to commit in the
evening, having to watch the tree and be on IRC and all.

-Scott

On Wed, Jan 19, 2011 at 6:01 PM, Albert Bodenhamer <abodenha@google.com>wrote:

> Local issues are worked out.  Once the tries pass we should be good to go.
>
>
> On Wed, Jan 19, 2011 at 5:30 PM, Albert Bodenhamer <abodenha@google.com>wrote:
>
>> Conflicts are resolved, new patch uploaded, try started.
>>
>> Unfortunately, I'm unable to build locally due to something that appears
>> unrelated (CEEE issue).
>>
>>
>> On Wed, Jan 19, 2011 at 2:57 PM, Albert Bodenhamer
<abodenha@google.com>wrote:
>>
>>> Ah, the joys of waiting for re-sync/re-build. :-) I'll try to have a new
>>> patch uploaded today.
>>>
>>> Thanks.
>>>
>>>
>>> On Wed, Jan 19, 2011 at 10:23 AM, <scottbyer@chromium.org> wrote:
>>>
>>>>
>>>> There was a conflict when merging up with trunk (one line).  It's
>>>> easiest if you
>>>> go ahead and do that merge and upload the revised patch, and I'll get it
>>>> committed.  Thanks!
>>>>
>>>> -Scott
>>>>
>>>>
>>>> On 2011/01/19 00:52:56, abodenha wrote:
>>>>
>>>>> Thanks!
>>>>>
>>>>
>>>>  On Tue, Jan 18, 2011 at 4:51 PM, <mailto:scottbyer@chromium.org>
>>>>> wrote:
>>>>>
>>>>
>>>>  > Sorry, this slipped our minds.  I'll commit in the morning.
>>>>> >
>>>>> >
>>>>> > On 2011/01/13 19:13:52, abodenha wrote:
>>>>> >
>>>>> >> Thanks.
>>>>> >>
>>>>> >
>>>>> >  Can one of you submit once the try completes?
>>>>> >>
>>>>> >
>>>>> >  On Thu, Jan 13, 2011 at 11:08 AM, <mailto:sanjeevr@chromium.org>
>>>>> wrote:
>>>>> >>
>>>>> >
>>>>> >  > LGTM
>>>>> >> >
>>>>> >> >
>>>>> >> > http://codereview.chromium.org/6038008/
>>>>> >> >
>>>>> >>
>>>>> >
>>>>> >
>>>>> >
>>>>> > http://codereview.chromium.org/6038008/
>>>>> >
>>>>>
>>>>
>>>>
>>>>
>>>> http://codereview.chromium.org/6038008/
>>>>
>>>
>>>
>>
>

[abodenha, 2011-01-20 06:10:51]

Sounds perfect. Thanks.
On Jan 19, 2011 6:59 PM, "Scott Byer" <scottbyer@chromium.org> wrote:
> I'll catch it first thing in the morning - I don't like to commit in the
> evening, having to watch the tree and be on IRC and all.
>
> -Scott
>
> On Wed, Jan 19, 2011 at 6:01 PM, Albert Bodenhamer <abodenha@google.com
>wrote:
>
>> Local issues are worked out. Once the tries pass we should be good to go.
>>
>>
>> On Wed, Jan 19, 2011 at 5:30 PM, Albert Bodenhamer <abodenha@google.com
>wrote:
>>
>>> Conflicts are resolved, new patch uploaded, try started.
>>>
>>> Unfortunately, I'm unable to build locally due to something that appears
>>> unrelated (CEEE issue).
>>>
>>>
>>> On Wed, Jan 19, 2011 at 2:57 PM, Albert Bodenhamer <abodenha@google.com
>wrote:
>>>
>>>> Ah, the joys of waiting for re-sync/re-build. :-) I'll try to have a
new
>>>> patch uploaded today.
>>>>
>>>> Thanks.
>>>>
>>>>
>>>> On Wed, Jan 19, 2011 at 10:23 AM, <scottbyer@chromium.org> wrote:
>>>>
>>>>>
>>>>> There was a conflict when merging up with trunk (one line). It's
>>>>> easiest if you
>>>>> go ahead and do that merge and upload the revised patch, and I'll get
it
>>>>> committed. Thanks!
>>>>>
>>>>> -Scott
>>>>>
>>>>>
>>>>> On 2011/01/19 00:52:56, abodenha wrote:
>>>>>
>>>>>> Thanks!
>>>>>>
>>>>>
>>>>> On Tue, Jan 18, 2011 at 4:51 PM, <mailto:scottbyer@chromium.org>
>>>>>> wrote:
>>>>>>
>>>>>
>>>>> > Sorry, this slipped our minds. I'll commit in the morning.
>>>>>> >
>>>>>> >
>>>>>> > On 2011/01/13 19:13:52, abodenha wrote:
>>>>>> >
>>>>>> >> Thanks.
>>>>>> >>
>>>>>> >
>>>>>> > Can one of you submit once the try completes?
>>>>>> >>
>>>>>> >
>>>>>> > On Thu, Jan 13, 2011 at 11:08 AM, <mailto:sanjeevr@chromium.org>
>>>>>> wrote:
>>>>>> >>
>>>>>> >
>>>>>> > > LGTM
>>>>>> >> >
>>>>>> >> >
>>>>>> >> > http://codereview.chromium.org/6038008/
>>>>>> >> >
>>>>>> >>
>>>>>> >
>>>>>> >
>>>>>> >
>>>>>> > http://codereview.chromium.org/6038008/
>>>>>> >
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> http://codereview.chromium.org/6038008/
>>>>>
>>>>
>>>>
>>>
>>

[Scott Byer, 2011-01-24 20:18:38]

http://codereview.chromium.org/6038008/diff/154001/chrome/browser/browser_mai...
File chrome/browser/browser_main.cc (right):

http://codereview.chromium.org/6038008/diff/154001/chrome/browser/browser_mai...
chrome/browser/browser_main.cc:11: #include "ui/base/l10n/l10n_util.h"
Go ahead and move this include line to it's proper sorted location.