Prevent leaking PDF data cross-origin

Prevent leaking PDF data cross-origin

BUG=520422
Committed: https://crrev.com/fff450abc4e2fb330ba700547a8e6a7b0fb90a6e
Cr-Commit-Position: refs/heads/master@{#345267}

[raymes, 2015-08-25 00:51:16]

raymes@chromium.org changed reviewers:
+ sammc@chromium.org

[Sam McNally, 2015-08-25 03:11:20]

https://codereview.chromium.org/1311973002/diff/40001/chrome/browser/pdf/pdf_...
File chrome/browser/pdf/pdf_extension_test.cc (right):

https://codereview.chromium.org/1311973002/diff/40001/chrome/browser/pdf/pdf_...
chrome/browser/pdf/pdf_extension_test.cc:48: CHECK(!*guest_out);
Why a CHECK?

https://codereview.chromium.org/1311973002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/pdf/pdf.js (right):

https://codereview.chromium.org/1311973002/diff/40001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf.js:826: window.dispatchEvent(new
CustomEvent('scripting-message-sent',
Could you add an extra listener on the plugin object in tests instead of always
dispatching this event?

https://codereview.chromium.org/1311973002/diff/40001/pdf/out_of_process_inst...
File pdf/out_of_process_instance.cc (right):

https://codereview.chromium.org/1311973002/diff/40001/pdf/out_of_process_inst...
pdf/out_of_process_instance.cc:308: // the plugin to be loaded in the extension
to avoid exposing sensitive APIs
And print preview?

https://codereview.chromium.org/1311973002/diff/40001/pdf/out_of_process_inst...
pdf/out_of_process_instance.cc:311: std::string document_url =
document_url_var.is_string() ?
if (!document_url_var.is_string())
  return false;

https://codereview.chromium.org/1311973002/diff/40001/pdf/out_of_process_inst...
pdf/out_of_process_instance.cc:316: !document_url.compare(0,
extension_url.size(), extension_url) ||
if (!base::StringPiece(document_url).starts_with(kChromeExtension) &&
    !base::StringPiece(document_url).starts_with(kChromePrint)) {
  return false;
}

[raymes, 2015-08-25 04:02:23]

https://codereview.chromium.org/1311973002/diff/40001/chrome/browser/pdf/pdf_...
File chrome/browser/pdf/pdf_extension_test.cc (right):

https://codereview.chromium.org/1311973002/diff/40001/chrome/browser/pdf/pdf_...
chrome/browser/pdf/pdf_extension_test.cc:48: CHECK(!*guest_out);
On 2015/08/25 03:11:20, Sam McNally wrote:
> Why a CHECK?

Done.

https://codereview.chromium.org/1311973002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/pdf/pdf.js (right):

https://codereview.chromium.org/1311973002/diff/40001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf.js:826: window.dispatchEvent(new
CustomEvent('scripting-message-sent',
On 2015/08/25 03:11:20, Sam McNally wrote:
> Could you add an extra listener on the plugin object in tests instead of
always
> dispatching this event?

Done.

https://codereview.chromium.org/1311973002/diff/40001/pdf/out_of_process_inst...
File pdf/out_of_process_instance.cc (right):

https://codereview.chromium.org/1311973002/diff/40001/pdf/out_of_process_inst...
pdf/out_of_process_instance.cc:308: // the plugin to be loaded in the extension
to avoid exposing sensitive APIs
On 2015/08/25 03:11:20, Sam McNally wrote:
> And print preview?

Done.

https://codereview.chromium.org/1311973002/diff/40001/pdf/out_of_process_inst...
pdf/out_of_process_instance.cc:311: std::string document_url =
document_url_var.is_string() ?
On 2015/08/25 03:11:20, Sam McNally wrote:
> if (!document_url_var.is_string())
>   return false;

Done.

https://codereview.chromium.org/1311973002/diff/40001/pdf/out_of_process_inst...
pdf/out_of_process_instance.cc:316: !document_url.compare(0,
extension_url.size(), extension_url) ||
On 2015/08/25 03:11:20, Sam McNally wrote:
> if (!base::StringPiece(document_url).starts_with(kChromeExtension) &&
>     !base::StringPiece(document_url).starts_with(kChromePrint)) {
>   return false;
> }

Done.

[raymes, 2015-08-25 04:03:38]

The CQ bit was checked by raymes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-25 04:04:13]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1311973002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1311973002/60001

[Sam McNally, 2015-08-25 04:08:38]

LGTM

https://codereview.chromium.org/1311973002/diff/60001/chrome/browser/pdf/pdf_...
File chrome/browser/pdf/pdf_extension_test.cc (right):

https://codereview.chromium.org/1311973002/diff/60001/chrome/browser/pdf/pdf_...
chrome/browser/pdf/pdf_extension_test.cc:46: bool
GetFirstGuestCallback(content::WebContents** guest_out,
I don't think "First" is accurate anymore.

https://codereview.chromium.org/1311973002/diff/60001/chrome/browser/pdf/pdf_...
chrome/browser/pdf/pdf_extension_test.cc:188:
"PDFViewer.prototype.sendScriptingMessage_;"
Indent this.

[commit-bot: I haz the power, 2015-08-25 04:40:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-25 04:40:09]

Dry run: This issue passed the CQ dry run.

[raymes, 2015-08-25 04:43:26]

https://codereview.chromium.org/1311973002/diff/60001/chrome/browser/pdf/pdf_...
File chrome/browser/pdf/pdf_extension_test.cc (right):

https://codereview.chromium.org/1311973002/diff/60001/chrome/browser/pdf/pdf_...
chrome/browser/pdf/pdf_extension_test.cc:46: bool
GetFirstGuestCallback(content::WebContents** guest_out,
On 2015/08/25 04:08:38, Sam McNally wrote:
> I don't think "First" is accurate anymore.

Done.

https://codereview.chromium.org/1311973002/diff/60001/chrome/browser/pdf/pdf_...
chrome/browser/pdf/pdf_extension_test.cc:188:
"PDFViewer.prototype.sendScriptingMessage_;"
On 2015/08/25 04:08:38, Sam McNally wrote:
> Indent this.

Done.

[raymes, 2015-08-25 04:43:29]

The CQ bit was checked by raymes@chromium.org

[raymes, 2015-08-25 04:43:29]

The patchset sent to the CQ was uploaded after l-g-t-m from sammc@chromium.org
Link to the patchset: https://codereview.chromium.org/1311973002/#ps80001
(title: " ")

[commit-bot: I haz the power, 2015-08-25 04:43:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1311973002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1311973002/80001

[commit-bot: I haz the power, 2015-08-25 06:02:14]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-08-25 06:03:27]

Patchset 5 (id:??) landed as
https://crrev.com/fff450abc4e2fb330ba700547a8e6a7b0fb90a6e
Cr-Commit-Position: refs/heads/master@{#345267}