Add crash key to record printed url

Crash in pdf compositor service was hard to debug without knowing the
content being printed. This CL added a new crash key "main-frame-url"
to record the main frame url being printed, so we can try to reproduce
the crashes with ease. Since the service is in a utility process,
it doesn't have the printed url info, such info has to be passed in
via IPC from browser process.

BUG=822218

Change-Id: Ic18c3c70ebb04738f0802be6c2e3f37b1ed4293d
Reviewed-on: https://chromium-review.googlesource.com/1005435
Commit-Queue: Wei Li <weili@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550342}

components/printing/browser/print_composite_client.cc

@@ -283,6 +283,7 @@ mojom::PdfCompositorPtr PrintCompositeClient::CreateCompositeRequest() {
   }
   mojom::PdfCompositorPtr compositor;
   connector_->BindInterface(mojom::kServiceName, &compositor);
+  compositor->SetWebContentsURL(web_contents()->GetLastCommittedURL());
   return compositor;
 }
 

components/printing/service/BUILD.gn

@@ -17,6 +17,7 @@ static_library("service") {
 
   deps = [
     "//base",
+    "//components/crash/core/common:crash_key",
     "//components/discardable_memory/client",
     "//components/discardable_memory/public/interfaces",
     "//content/public/child",
@@ -56,6 +57,7 @@ if (enable_basic_printing || enable_print_preview) {
       ":service",
       "//base/test:test_support",
       "//cc/paint:paint",
+      "//components/crash/core/common:crash_key",
       "//components/printing/service/public/interfaces",
       "//mojo/common",
       "//services/service_manager/public/cpp:service_test_support",

components/printing/service/DEPS

 include_rules = [
   "+cc/paint",
+  "+components/crash/core/common/crash_key.h",
   "+components/discardable_memory/client",
   "+content/public/child",  # Windows direct write proxy access.
   "+content/public/utility",

components/printing/service/pdf_compositor_impl.cc

@@ -9,6 +9,7 @@
 #include "base/logging.h"
 #include "base/memory/shared_memory_handle.h"
 #include "base/stl_util.h"
+#include "components/crash/core/common/crash_key.h"
 #include "components/printing/service/public/cpp/pdf_service_mojo_types.h"
 #include "components/printing/service/public/cpp/pdf_service_mojo_utils.h"
 #include "mojo/public/cpp/system/platform_handle.h"
@@ -96,6 +97,13 @@ void PdfCompositorImpl::CompositeDocumentToPdf(
                            std::move(callback));
 }
 
+void PdfCompositorImpl::SetWebContentsURL(const GURL& url) {
+  // Record the most recent url we tried to print. This should be sufficient
+  // for users using print preview by default.
+  static crash_reporter::CrashKeyString<1024> crash_key("main-frame-url");
+  crash_key.Set(url.spec());
+}
+
 void PdfCompositorImpl::UpdateRequestsWithSubframeInfo(
     uint64_t frame_guid,
     const std::vector<uint64_t>& pending_subframes) {

components/printing/service/pdf_compositor_impl.h

@@ -49,6 +49,7 @@ class PdfCompositorImpl : public mojom::PdfCompositor {
       mojo::ScopedSharedBufferHandle serialized_content,
       const ContentToFrameMap& subframe_content_map,
       mojom::PdfCompositor::CompositeDocumentToPdfCallback callback) override;
+  void SetWebContentsURL(const GURL& url) override;
 
  protected:
   // This is the uniform underlying type for both

components/printing/service/pdf_compositor_impl_unittest.cc

@@ -7,6 +7,7 @@
 #include "base/callback.h"
 #include "base/run_loop.h"
 #include "base/test/scoped_task_environment.h"
+#include "components/crash/core/common/crash_key.h"
 #include "components/printing/service/pdf_compositor_impl.h"
 #include "components/printing/service/public/cpp/pdf_service_mojo_types.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -61,6 +62,22 @@ class PdfCompositorImplTest : public testing::Test {
   bool is_ready_;
 };
 
+class PdfCompositorImplCrashKeyTest : public PdfCompositorImplTest {
+ public:
+  PdfCompositorImplCrashKeyTest() {}
+  ~PdfCompositorImplCrashKeyTest() override {}
+
+  void SetUp() override {
+    crash_reporter::ResetCrashKeysForTesting();
+    crash_reporter::InitializeCrashKeys();
+  }
+
+  void TearDown() override { crash_reporter::ResetCrashKeysForTesting(); }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(PdfCompositorImplCrashKeyTest);
+};
+
 TEST_F(PdfCompositorImplTest, IsReadyToComposite) {
   PdfCompositorImpl impl("unittest", nullptr);
   // Frame 2 and 3 are painted.
@@ -345,4 +362,13 @@ TEST_F(PdfCompositorImplTest, NotifyUnavailableSubframe) {
   testing::Mock::VerifyAndClearExpectations(&impl);
 }
 
+TEST_F(PdfCompositorImplCrashKeyTest, SetCrashKey) {
+  PdfCompositorImpl impl("unittest", nullptr);
+  std::string url_str("https://www.example.com/");
+  GURL url(url_str);
+  impl.SetWebContentsURL(url);
+
+  EXPECT_EQ(crash_reporter::GetCrashKeyValue("main-frame-url"), url_str);
+}
+
 }  // namespace printing

components/printing/service/public/interfaces/BUILD.gn

@@ -8,4 +8,7 @@ mojom("interfaces") {
   sources = [
     "pdf_compositor.mojom",
   ]
+  public_deps = [
+    "//url/mojom:url_mojom_gurl",
+  ]
 }

components/printing/service/public/interfaces/pdf_compositor.mojom

@@ -4,6 +4,8 @@
 
 module printing.mojom;
 
+import "url/mojom/url.mojom";
+
 const string kServiceName = "pdf_compositor";
 
 interface PdfCompositor {
@@ -23,7 +25,7 @@ interface PdfCompositor {
   // |frame_guid| is this subframe's global unique id.
   NotifyUnavailableSubframe(uint64 frame_guid);
 
-  // Add the content of a subframe for composition.
+  // Adds the content of a subframe for composition.
   // |frame_guid| is this subframe's global unique id.
   // |serialized_content| points to a buffer of a serialized Skia picture which
   //                      has the painted content of this frame.
@@ -32,7 +34,7 @@ interface PdfCompositor {
   AddSubframeContent(uint64 frame_guid, handle<shared_buffer> serialized_content,
                      map<uint32, uint64> subframe_content_info);
 
-  // Request to composite a page and convert it into a PDF file.
+  // Requests to composite a page and convert it into a PDF file.
   // |frame_guid| is the global unique id of the frame to be composited.
   // |page_num| is zero-based sequence number of page.
   // |sk_handle| points to a buffer of a Skia MultiPictureDocument which has
@@ -44,10 +46,14 @@ interface PdfCompositor {
                      map<uint32, uint64> subframe_content_info)
       => (Status status, handle<shared_buffer>? pdf_handle);
 
-  // Request to composite the entire document and convert it into a PDF file.
+  // Requests to composite the entire document and convert it into a PDF file.
   // All the arguments carry the same meaning as CompositePageToPdf() above,
   // except this call doesn't have |page_num|.
   CompositeDocumentToPdf(uint64 frame_guid, handle<shared_buffer> sk_handle,
                          map<uint32, uint64> subframe_content_info)
       => (Status status, handle<shared_buffer>? pdf_handle);
+
+  // Sets the URL which is committed in the main frame of the WebContents,
+  // for use in crash diagnosis.
+  SetWebContentsURL(url.mojom.Url url);
 };