Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(13)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 5947002: As the first step in an effort to improve robustness of the cloud print proxy... (Closed)

Created:
10 years ago by sanjeevr
Modified:
9 years, 7 months ago
Reviewers:
jam, gene, Scott Byer
CC:
chromium-reviews, Paweł Hajdan Jr.
Visibility:
Public.

Description

As the first step in an effort to improve robustness of the cloud print proxy, we fetch printer capabilities and defaults in a child process so that printer driver crashes do not crash the entire proxy. BUG=None TEST=Registration of printers, printer update in Cloud Print Proxy. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=69899

Patch Set 1 #

Patch Set 2 : Fixed a lint error #

Patch Set 3 : Synced to latest and merged #

Patch Set 4 : Fixed compiler errors #

Patch Set 5 : Fixed Mac/Linux compile error #

Total comments: 10

Patch Set 6 : Review comments #

Total comments: 2

Patch Set 7 : Review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+441 lines, -119 lines) Patch
M chrome/common/common_param_traits.h View 1 2 3 4 5 2 chunks +9 lines, -0 lines 0 comments Download
M chrome/common/common_param_traits.cc View 1 2 3 4 5 2 chunks +31 lines, -0 lines 0 comments Download
M chrome/common/common_param_traits_unittest.cc View 1 2 3 4 5 2 chunks +22 lines, -0 lines 0 comments Download
M chrome/common/utility_messages_internal.h View 1 2 3 4 5 6 3 chunks +20 lines, -0 lines 0 comments Download
M chrome/service/cloud_print/cloud_print_proxy_backend.cc View 1 2 3 4 5 2 chunks +85 lines, -61 lines 0 comments Download
M chrome/service/cloud_print/print_system.h View 1 2 3 4 5 3 chunks +10 lines, -3 lines 0 comments Download
M chrome/service/cloud_print/print_system_cups.cc View 1 2 3 4 5 6 chunks +61 lines, -26 lines 0 comments Download
M chrome/service/cloud_print/print_system_win.cc View 1 2 3 4 5 3 chunks +74 lines, -5 lines 0 comments Download
M chrome/service/cloud_print/printer_job_handler.h View 1 2 3 4 5 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/service/cloud_print/printer_job_handler.cc View 1 2 3 4 5 3 chunks +35 lines, -11 lines 0 comments Download
M chrome/service/service_child_process_host.h View 1 2 3 4 5 1 chunk +5 lines, -1 line 0 comments Download
M chrome/service/service_child_process_host.cc View 1 2 3 4 5 2 chunks +12 lines, -1 line 0 comments Download
M chrome/service/service_utility_process_host.h View 1 2 3 4 5 5 chunks +23 lines, -2 lines 0 comments Download
M chrome/service/service_utility_process_host.cc View 1 2 3 4 5 4 chunks +17 lines, -3 lines 0 comments Download
M chrome/utility/utility_main.cc View 1 2 3 4 5 1 chunk +8 lines, -6 lines 0 comments Download
M chrome/utility/utility_thread.h View 1 2 3 4 5 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/utility/utility_thread.cc View 1 2 3 4 5 2 chunks +17 lines, -0 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
sanjeevr
10 years ago (2010-12-17 01:12:29 UTC) #1
Scott Byer
LGTM w/ nits. http://codereview.chromium.org/5947002/diff/52001/chrome/service/service_child_process_host.h File chrome/service/service_child_process_host.h (right): http://codereview.chromium.org/5947002/diff/52001/chrome/service/service_child_process_host.h#newcode26 chrome/service/service_child_process_host.h:26: bool Launch(CommandLine* cmd_line, Please document that ...
10 years ago (2010-12-20 21:02:28 UTC) #2
gene
http://codereview.chromium.org/5947002/diff/52001/chrome/service/cloud_print/cloud_print_proxy_backend.cc File chrome/service/cloud_print/cloud_print_proxy_backend.cc (right): http://codereview.chromium.org/5947002/diff/52001/chrome/service/cloud_print/cloud_print_proxy_backend.cc#newcode444 chrome/service/cloud_print/cloud_print_proxy_backend.cc:444: OnReceivePrinterCaps(true, Use PostTask here http://codereview.chromium.org/5947002/diff/52001/chrome/service/service_utility_process_host.cc File chrome/service/service_utility_process_host.cc (right): http://codereview.chromium.org/5947002/diff/52001/chrome/service/service_utility_process_host.cc#newcode108 ...
10 years ago (2010-12-20 22:52:30 UTC) #3
jam
http://codereview.chromium.org/5947002/diff/52001/chrome/common/utility_messages_internal.h File chrome/common/utility_messages_internal.h (right): http://codereview.chromium.org/5947002/diff/52001/chrome/common/utility_messages_internal.h#newcode69 chrome/common/utility_messages_internal.h:69: std::string) // printer name style nit: *_messages_internal.h files put ...
10 years ago (2010-12-20 23:10:37 UTC) #4
sanjeevr
Made the changes and uploaded. http://codereview.chromium.org/5947002/diff/52001/chrome/common/utility_messages_internal.h File chrome/common/utility_messages_internal.h (right): http://codereview.chromium.org/5947002/diff/52001/chrome/common/utility_messages_internal.h#newcode69 chrome/common/utility_messages_internal.h:69: std::string) // printer name ...
10 years ago (2010-12-21 21:28:45 UTC) #5
jam
http://codereview.chromium.org/5947002/diff/65001/chrome/common/utility_messages_internal.h File chrome/common/utility_messages_internal.h (right): http://codereview.chromium.org/5947002/diff/65001/chrome/common/utility_messages_internal.h#newcode143 chrome/common/utility_messages_internal.h:143: std::string, // printer name here and below too
10 years ago (2010-12-21 22:04:40 UTC) #6
sanjeevr
http://codereview.chromium.org/5947002/diff/65001/chrome/common/utility_messages_internal.h File chrome/common/utility_messages_internal.h (right): http://codereview.chromium.org/5947002/diff/65001/chrome/common/utility_messages_internal.h#newcode143 chrome/common/utility_messages_internal.h:143: std::string, // printer name On 2010/12/21 22:04:41, John Abd-El-Malek ...
10 years ago (2010-12-21 22:32:39 UTC) #7
gene
lgtm On 2010/12/21 22:32:39, sanjeevr wrote: > http://codereview.chromium.org/5947002/diff/65001/chrome/common/utility_messages_internal.h > File chrome/common/utility_messages_internal.h (right): > > http://codereview.chromium.org/5947002/diff/65001/chrome/common/utility_messages_internal.h#newcode143 ...
10 years ago (2010-12-21 22:40:18 UTC) #8
jam
10 years ago (2010-12-21 22:40:27 UTC) #9 
thanks, lgtm

On Tue, Dec 21, 2010 at 2:32 PM, <sanjeevr@chromium.org> wrote:

>
>
>
http://codereview.chromium.org/5947002/diff/65001/chrome/common/utility_messa...
> File chrome/common/utility_messages_internal.h (right):
>
>
>
http://codereview.chromium.org/5947002/diff/65001/chrome/common/utility_messa...
> chrome/common/utility_messages_internal.h:143: std::string,    //
> printer name
> On 2010/12/21 22:04:41, John Abd-El-Malek wrote:
>
>> here and below too
>>
>
> Done.
>
>
> http://codereview.chromium.org/5947002/
>

Powered by Google App Engine
This is Rietveld 408576698