Get signals working in the EXTRA_DATA section of MHTML

content/browser/download/mhtml_extra_data.h

Index: content/browser/download/mhtml_extra_data.h
diff --git a/content/browser/download/mhtml_extra_data.h b/content/browser/download/mhtml_extra_data.h
new file mode 100644
index 0000000000000000000000000000000000000000..f3e13502236af5b0e98a6d51cbb51c82ddd855fd
--- /dev/null
+++ b/content/browser/download/mhtml_extra_data.h
@@ -0,0 +1,23 @@
+// Copyright (c) 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+#ifndef CONTENT_BROWSER_DOWNLOAD_MHTML_EXTRA_DATA_H_
+#define CONTENT_BROWSER_DOWNLOAD_MHTML_EXTRA_DATA_H_
+
+#include <string>
+
+#include "base/supports_user_data.h"
+
+namespace content {

[Dmitry Titov, 2017/03/16 02:28:20]

Not sure why this file should live in content... This can be part of
component/offline_pages/offliner.h for example. It doesn't have to live in
content/browser...

Crazy idea: in the future, this can be a SnapshotController as well...

[Pete Williamson, 2017/03/17 23:28:32]

We dicussed this in person, and agreed to move the file into
mhtml_generation_manager.h.  The implementation got moved into
mhtml_generation_manager.cc

I like the idea of moving this into our SnapshotController/SignalCollector.

+
+const void* kMHTMLExtraDataKey = (void*)0x14725138;

[Pete Williamson, 2017/03/15 22:38:22]

This is a kind of clunk way to do a key, just picking some arbitrary number.  I
tried previously using a string here, but it got put into different compilation
units, which means it address was not constant.  Reviewers, any suggestions on
how to make this nicer?

[Dmitry Titov, 2017/03/16 02:28:20]

It is usually done like this:

static int kFooKey;

...
SetUserData(&kFooKey, data);

[fgorski, 2017/03/16 17:46:31]

It was in different compilation unit because you put it in .h file instead of
.cc file...

It is best if you actually declare it in .cc and add a static method to this
class that will know how to get/set itself from/on web_contents... Otherwise we
will run into trouble.


E.g.:
https://cs.chromium.org/chromium/src/components/offline_pages/core/downloads/...

[Pete Williamson, 2017/03/17 23:28:32]

Done.

[Pete Williamson, 2017/03/17 23:28:32]

Done.

+
+struct MHTMLExtraData : public base::SupportsUserData::Data {
+  MHTMLExtraData(std::string data) : data_string(data) {}
+  ~MHTMLExtraData() override {}
+  std::string data_string;
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_DOWNLOAD_MHTML_EXTRA_DATA_H_