Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
791133006:
Delegates for the printing component (Closed)
DescriptionDelegates for the printing component
PrintWebViewHelperDelegate is introduced to remove PrintWebViewHelper's
dependencies on //chrome.
Some dependencies on //chrome are still in the file and will be moved
to //components at the same time as the file
BUG=446509
Committed: https://crrev.com/9205d0800cfdfc1e74d70121265992c2bbfa1d12
Cr-Commit-Position: refs/heads/master@{#310359}
Patch Set 1 #
Total comments: 18
Patch Set 2 : Using initialisation parameters instead of delegate methods #
Total comments: 18
Patch Set 3 : Pass extensionId as parameter, PWVH owns delegate #Patch Set 4 : rebase #
Total comments: 7
Patch Set 5 : pdf_extension_id as std::string, changed back the delegate ownership #
Total comments: 9
Patch Set 6 : PWVH owns its delegate, getPdfElt back in the delegate #Patch Set 7 : Removed an unused include #
Total comments: 1
Patch Set 8 : nit fix #
Messages
Total messages: 22 (4 generated)
I also can be done by having a default implementation of those functions directly in PrintWebViewHelper and subclassing it in chrome for the specialized behavior... https://codereview.chromium.org/791133006/diff/1/chrome/renderer/chrome_conte... File chrome/renderer/chrome_content_renderer_client.cc (right): https://codereview.chromium.org/791133006/diff/1/chrome/renderer/chrome_conte... chrome/renderer/chrome_content_renderer_client.cc:511: scoped_ptr<printing::ChromePrintWebViewHelperDelegate>( I was wondering here why the delegates are all owned by ChromeContentRendererClient. Isn't it more reliable to have them owned by the delegate? That way at least we know the object is always there instead of just assuming it? https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/chr... File chrome/renderer/printing/chrome_print_web_view_helper_delegate.cc (right): https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/chr... chrome/renderer/printing/chrome_print_web_view_helper_delegate.cc:64: base::StringValue ChromePrintWebViewHelperDelegate::GetPrintPreviewHtml() { Will probably end up with the whole function in the |#if defined(ENABLE_PRINT_PREVIEW)|
vitalybuka@chromium.org changed reviewers: + vitalybuka@chromium.org
https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/chr... File chrome/renderer/printing/chrome_print_web_view_helper_delegate.cc (right): https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/chr... chrome/renderer/printing/chrome_print_web_view_helper_delegate.cc:35: render_view->GetMainRenderFrame())) there is content::RenderView::Send please rename GetScriptedPrintCancelMessage to CancelPrerender and do here: if (render_view) return render_view->Send(message); delete message; https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/chr... chrome/renderer/printing/chrome_print_web_view_helper_delegate.cc:42: return switches::OutOfProcessPdfEnabled(); OutOfProcessPdfEnabled and PrintPreviewDisabled should be changed after PrintWebViewHelper is created we should make them just arguments of PrintWebViewHelper constructor. https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/chr... chrome/renderer/printing/chrome_print_web_view_helper_delegate.cc:50: if (url.SchemeIs(extensions::kExtensionScheme) && seems ChromePrintWebViewHelperDelegate::GetPdfElement can be removed from delegate https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/chr... chrome/renderer/printing/chrome_print_web_view_helper_delegate.cc:64: base::StringValue ChromePrintWebViewHelperDelegate::GetPrintPreviewHtml() { seems ChromePrintWebViewHelperDelegate::GetPrintPreviewHtml also can be removed from delegate https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/pri... File chrome/renderer/printing/print_web_view_helper.cc (left): https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/pri... chrome/renderer/printing/print_web_view_helper.cc:417: if (url.SchemeIs(extensions::kExtensionScheme) && Is this because of extensions? Seems components are allowed to deppend on /src/extensions/ Ex. components/renderer_context_menu/context_menu_content_type.cc https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/pri... chrome/renderer/printing/print_web_view_helper.cc:495: // Load page with script to avoid async operations. this resource has nothing chrome specific so it should be moved to components as well check how it's done for https://code.google.com/p/chromium/codesearch#search/&q=IDR_TRANSLATE_JS&sq=p... https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/pri... chrome/renderer/printing/print_web_view_helper.cc:806: if (CommandLine::ForCurrentProcess()->HasSwitch( this switch could go to components not sure yet if we will have other switches there https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/pri... File chrome/renderer/printing/print_web_view_helper_delegate.h (right): https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/pri... chrome/renderer/printing/print_web_view_helper_delegate.h:28: class PrintWebViewHelperDelegate { it's a trivial interface Could we make it a nested class? class PrintWebViewHelper { class Delegate { } }
https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/chr... File chrome/renderer/printing/chrome_print_web_view_helper_delegate.cc (right): https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/chr... chrome/renderer/printing/chrome_print_web_view_helper_delegate.cc:35: render_view->GetMainRenderFrame())) On 2015/01/05 19:57:03, Vitaly Buka wrote: > there is content::RenderView::Send > please rename GetScriptedPrintCancelMessage to CancelPrerender > > and do here: > if (render_view) > return render_view->Send(message); > delete message; I'm not sure to understand the second part of the comment. Is the new implementation ok? https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/chr... chrome/renderer/printing/chrome_print_web_view_helper_delegate.cc:42: return switches::OutOfProcessPdfEnabled(); On 2015/01/05 19:57:03, Vitaly Buka wrote: > OutOfProcessPdfEnabled and PrintPreviewDisabled should be changed after > PrintWebViewHelper is created > we should make them just arguments of PrintWebViewHelper constructor. Done. https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/chr... chrome/renderer/printing/chrome_print_web_view_helper_delegate.cc:50: if (url.SchemeIs(extensions::kExtensionScheme) && On 2015/01/05 19:57:03, Vitaly Buka wrote: > seems ChromePrintWebViewHelperDelegate::GetPdfElement can be removed from > delegate Done. https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/chr... chrome/renderer/printing/chrome_print_web_view_helper_delegate.cc:64: base::StringValue ChromePrintWebViewHelperDelegate::GetPrintPreviewHtml() { On 2015/01/05 19:57:03, Vitaly Buka wrote: > seems ChromePrintWebViewHelperDelegate::GetPrintPreviewHtml also can be removed > from delegate Done. https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/pri... File chrome/renderer/printing/print_web_view_helper.cc (left): https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/pri... chrome/renderer/printing/print_web_view_helper.cc:417: if (url.SchemeIs(extensions::kExtensionScheme) && On 2015/01/05 19:57:03, Vitaly Buka wrote: > Is this because of extensions? > Seems components are allowed to deppend on /src/extensions/ > Ex. components/renderer_context_menu/context_menu_content_type.cc That was about the extensionId but apparently it's copied in many places already. I guess it's ok to have it here also. https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/pri... chrome/renderer/printing/print_web_view_helper.cc:495: // Load page with script to avoid async operations. On 2015/01/05 19:57:03, Vitaly Buka wrote: > this resource has nothing chrome specific so it should be moved to components as > well > > check how it's done for > > https://code.google.com/p/chromium/codesearch#search/&q=IDR_TRANSLATE_JS&sq=p... Acknowledged. https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/pri... chrome/renderer/printing/print_web_view_helper.cc:806: if (CommandLine::ForCurrentProcess()->HasSwitch( On 2015/01/05 19:57:03, Vitaly Buka wrote: > this switch could go to components > not sure yet if we will have other switches there Replaced with a constructor argument https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/pri... File chrome/renderer/printing/print_web_view_helper_delegate.h (right): https://codereview.chromium.org/791133006/diff/1/chrome/renderer/printing/pri... chrome/renderer/printing/print_web_view_helper_delegate.h:28: class PrintWebViewHelperDelegate { On 2015/01/05 19:57:03, Vitaly Buka wrote: > it's a trivial interface > Could we make it a nested class? > > class PrintWebViewHelper { > class Delegate { > } > } Done. https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... File chrome/renderer/printing/print_web_view_helper.cc (left): https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.cc:20: #include "chrome/common/print_messages.h" Will move to //components/printing/common/print_messages.h https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.cc:22: #include "chrome/grit/browser_resources.h" The used variable will be moved to //components/printing
lgtm https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... File chrome/renderer/printing/print_web_view_helper.h (right): https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.h:79: const bool out_of_process_pdf_enabled, "const bool" -> "bool"
vitalybuka@chromium.org changed reviewers: + thestig@chromium.org
+thestig for chrome/
https://codereview.chromium.org/791133006/diff/20001/chrome/chrome_renderer.gypi File chrome/chrome_renderer.gypi (right): https://codereview.chromium.org/791133006/diff/20001/chrome/chrome_renderer.g... chrome/chrome_renderer.gypi:238: 'renderer/printing/chrome_print_web_view_helper_delegate.h', nit: alphabetical order please. https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/chrome_c... File chrome/renderer/chrome_content_renderer_client.cc (right): https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/chrome_c... chrome/renderer/chrome_content_renderer_client.cc:515: print_web_view_helper_delegate_.get()); Does the delegate outlive the PrintWebViewHelper? https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... File chrome/renderer/printing/chrome_print_web_view_helper_delegate.cc (right): https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... chrome/renderer/printing/chrome_print_web_view_helper_delegate.cc:1: // Copyright (c) 2014 The Chromium Authors. All rights reserved. It's 2015. :) https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... File chrome/renderer/printing/chrome_print_web_view_helper_delegate.h (right): https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... chrome/renderer/printing/chrome_print_web_view_helper_delegate.h:1: // Copyright (c) 2015 The Chromium Authors. All rights reserved. nit: no (c) https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... File chrome/renderer/printing/print_web_view_helper.cc (right): https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.cc:53: static const char kPdfExtensionId[] = "mhjfbmdgcfjbbpaeojofohoefgiehjai"; Can we pass this in, instead of duplicating the value? https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... File chrome/renderer/printing/print_web_view_helper.h (right): https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.h:78: explicit PrintWebViewHelper(content::RenderView* render_view, explicit no longer needed https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.h:345: Delegate* delegate_; This can be a Delegate* const
https://codereview.chromium.org/791133006/diff/20001/chrome/chrome_renderer.gypi File chrome/chrome_renderer.gypi (right): https://codereview.chromium.org/791133006/diff/20001/chrome/chrome_renderer.g... chrome/chrome_renderer.gypi:238: 'renderer/printing/chrome_print_web_view_helper_delegate.h', On 2015/01/06 04:11:32, Lei Zhang wrote: > nit: alphabetical order please. Done. https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/chrome_c... File chrome/renderer/chrome_content_renderer_client.cc (right): https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/chrome_c... chrome/renderer/chrome_content_renderer_client.cc:515: print_web_view_helper_delegate_.get()); On 2015/01/06 04:11:32, Lei Zhang wrote: > Does the delegate outlive the PrintWebViewHelper? Yes. From my understanding, when it is destroyed, render_view unregisters PrintWebViewHelper which in turn also destroys itself. CCRendererClient outlives them. The same system is used for other delegates in the files. I just did it for consistency. That being said, I am also more comfortable with transferring the ownership to PrintWebViewHelper (as done in PatchSet1). Is the new version better? https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... File chrome/renderer/printing/chrome_print_web_view_helper_delegate.cc (right): https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... chrome/renderer/printing/chrome_print_web_view_helper_delegate.cc:1: // Copyright (c) 2014 The Chromium Authors. All rights reserved. On 2015/01/06 04:11:32, Lei Zhang wrote: > It's 2015. :) Done. https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... File chrome/renderer/printing/chrome_print_web_view_helper_delegate.h (right): https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... chrome/renderer/printing/chrome_print_web_view_helper_delegate.h:1: // Copyright (c) 2015 The Chromium Authors. All rights reserved. On 2015/01/06 04:11:32, Lei Zhang wrote: > nit: no (c) Done. https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... File chrome/renderer/printing/print_web_view_helper.cc (right): https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.cc:53: static const char kPdfExtensionId[] = "mhjfbmdgcfjbbpaeojofohoefgiehjai"; On 2015/01/06 04:11:32, Lei Zhang wrote: > Can we pass this in, instead of duplicating the value? Done. https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... File chrome/renderer/printing/print_web_view_helper.h (right): https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.h:78: explicit PrintWebViewHelper(content::RenderView* render_view, On 2015/01/06 04:11:32, Lei Zhang wrote: > explicit no longer needed Done. https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.h:79: const bool out_of_process_pdf_enabled, On 2015/01/06 03:46:16, Vitaly Buka wrote: > "const bool" -> "bool" Done. https://codereview.chromium.org/791133006/diff/60001/chrome/renderer/printing... File chrome/renderer/printing/print_web_view_helper.cc (right): https://codereview.chromium.org/791133006/diff/60001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.cc:410: const char* pdf_extension_id) { Would it be better to make this function a method of PrintWebViewHelper instead, to avoid passing |pdf_extension_id|?
https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/chrome_c... File chrome/renderer/chrome_content_renderer_client.cc (right): https://codereview.chromium.org/791133006/diff/20001/chrome/renderer/chrome_c... chrome/renderer/chrome_content_renderer_client.cc:515: print_web_view_helper_delegate_.get()); I'd prefer to transfer ownership at it was in the first patch. On 2015/01/06 16:35:30, dgn wrote: > On 2015/01/06 04:11:32, Lei Zhang wrote: > > Does the delegate outlive the PrintWebViewHelper? > > Yes. From my understanding, when it is destroyed, render_view unregisters > PrintWebViewHelper which in turn also destroys itself. CCRendererClient outlives > them. The same system is used for other delegates in the files. I just did it > for consistency. That being said, I am also more comfortable with transferring > the ownership to PrintWebViewHelper (as done in PatchSet1). Is the new version > better? https://codereview.chromium.org/791133006/diff/60001/chrome/renderer/printing... File chrome/renderer/printing/print_web_view_helper.cc (right): https://codereview.chromium.org/791133006/diff/60001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.cc:410: const char* pdf_extension_id) { It's fine to keep as is On 2015/01/06 16:35:31, dgn wrote: > Would it be better to make this function a method of PrintWebViewHelper instead, > to avoid passing |pdf_extension_id|? please use const std::string& if you don'e have rely important reasons to not to do so. https://codereview.chromium.org/791133006/diff/60001/chrome/renderer/printing... File chrome/renderer/printing/print_web_view_helper.h (right): https://codereview.chromium.org/791133006/diff/60001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.h:80: const char* pdf_extension_id, please use const std::string& https://codereview.chromium.org/791133006/diff/60001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.h:347: const char* pdf_extension_id_; use std::string here you can't be sure that it's not a temp buffer.
https://codereview.chromium.org/791133006/diff/60001/chrome/renderer/printing... File chrome/renderer/printing/print_web_view_helper.cc (right): https://codereview.chromium.org/791133006/diff/60001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.cc:410: const char* pdf_extension_id) { On 2015/01/06 19:29:24, Vitaly Buka wrote: > It's fine to keep as is > > On 2015/01/06 16:35:31, dgn wrote: > > Would it be better to make this function a method of PrintWebViewHelper > instead, > > to avoid passing |pdf_extension_id|? > > please use const std::string& if you don'e have rely important reasons to not to > do so. > Done. https://codereview.chromium.org/791133006/diff/60001/chrome/renderer/printing... File chrome/renderer/printing/print_web_view_helper.h (right): https://codereview.chromium.org/791133006/diff/60001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.h:80: const char* pdf_extension_id, On 2015/01/06 19:29:24, Vitaly Buka wrote: > please use const std::string& Done. https://codereview.chromium.org/791133006/diff/60001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.h:347: const char* pdf_extension_id_; On 2015/01/06 19:29:24, Vitaly Buka wrote: > use std::string here > you can't be sure that it's not a temp buffer. Done.
lgtm
On 2015/01/06 22:44:08, Lei Zhang wrote: > lgtm just a few questions to better understand the design.
sgurun@chromium.org changed reviewers: + sgurun@chromium.org
https://codereview.chromium.org/791133006/diff/80001/chrome/renderer/chrome_c... File chrome/renderer/chrome_content_renderer_client.cc (right): https://codereview.chromium.org/791133006/diff/80001/chrome/renderer/chrome_c... chrome/renderer/chrome_content_renderer_client.cc:522: extension_misc::kPdfExtensionId, will android webview pass an empty string here? https://codereview.chromium.org/791133006/diff/80001/chrome/renderer/printing... File chrome/renderer/printing/print_web_view_helper.cc (left): https://codereview.chromium.org/791133006/diff/80001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.cc:839: Send(new ChromeViewHostMsg_CancelPrerenderForPrinting(routing_id())); Will this message eventually live in chrome or will we move all ipc messages to content layer? https://codereview.chromium.org/791133006/diff/80001/chrome/renderer/printing... File chrome/renderer/printing/print_web_view_helper.cc (right): https://codereview.chromium.org/791133006/diff/80001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.cc:19: #include "chrome/grit/browser_resources.h" just a reminder: android webview and content cannot depend on chrome. https://codereview.chromium.org/791133006/diff/80001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.cc:413: if (url.SchemeIs(extensions::kExtensionScheme) && will this logic only live in chrome eventually?
https://codereview.chromium.org/791133006/diff/80001/chrome/renderer/printing... File chrome/renderer/printing/print_web_view_helper.cc (right): https://codereview.chromium.org/791133006/diff/80001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.cc:413: if (url.SchemeIs(extensions::kExtensionScheme) && On 2015/01/06 22:57:43, sgurun wrote: > will this logic only live in chrome eventually? Nicolas, seems it was my mistake to ask you to remove delegate_->GetPdfElement() If it's required by desktop Chrome only, we can have it as part of delegate.
https://codereview.chromium.org/791133006/diff/80001/chrome/renderer/chrome_c... File chrome/renderer/chrome_content_renderer_client.cc (right): https://codereview.chromium.org/791133006/diff/80001/chrome/renderer/chrome_c... chrome/renderer/chrome_content_renderer_client.cc:522: extension_misc::kPdfExtensionId, On 2015/01/06 22:57:43, sgurun wrote: > will android webview pass an empty string here? That was the plan, yes. It's not needed anymore, webview will have to implement GetPdfElement and can return a new element such that isNull() is true https://codereview.chromium.org/791133006/diff/80001/chrome/renderer/printing... File chrome/renderer/printing/print_web_view_helper.cc (left): https://codereview.chromium.org/791133006/diff/80001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.cc:839: Send(new ChromeViewHostMsg_CancelPrerenderForPrinting(routing_id())); On 2015/01/06 22:57:43, sgurun wrote: > Will this message eventually live in chrome or will we move all ipc messages to > content layer? I don't know if there's something planned for the IPC messages. Since the prerender code here is still in chrome I just moved the calls to the delegate. The webview implementation of the function could just return |false|. https://codereview.chromium.org/791133006/diff/80001/chrome/renderer/printing... File chrome/renderer/printing/print_web_view_helper.cc (right): https://codereview.chromium.org/791133006/diff/80001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.cc:19: #include "chrome/grit/browser_resources.h" On 2015/01/06 22:57:43, sgurun wrote: > just a reminder: android webview and content cannot depend on chrome. Yes, the resources used here will be moved to the component when this file is also moved https://codereview.chromium.org/791133006/diff/80001/chrome/renderer/printing... chrome/renderer/printing/print_web_view_helper.cc:413: if (url.SchemeIs(extensions::kExtensionScheme) && On 2015/01/07 00:21:24, Vitaly Buka wrote: > On 2015/01/06 22:57:43, sgurun wrote: > > will this logic only live in chrome eventually? > > Nicolas, seems it was my mistake to ask you to remove delegate_->GetPdfElement() > > If it's required by desktop Chrome only, we can have it as part of delegate. > > > Ok, then pdf_extension_id is not needed anymore, which will avoid having to pass it from webview.
lgtm https://codereview.chromium.org/791133006/diff/120001/chrome/renderer/chrome_... File chrome/renderer/chrome_content_renderer_client.cc (right): https://codereview.chromium.org/791133006/diff/120001/chrome/renderer/chrome_... chrome/renderer/chrome_content_renderer_client.cc:521: new ChromePrintWebViewHelperDelegate())); two more spaces before "new".
The CQ bit was checked by dgn@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/791133006/140001
Message was sent while issue was closed.
Committed patchset #8 (id:140001)
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/9205d0800cfdfc1e74d70121265992c2bbfa1d12 Cr-Commit-Position: refs/heads/master@{#310359} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
