Run ChromeVox PDF extractor within Chrome's PDF extension

Run ChromeVox PDF extractor within Chrome's PDF extension

Previously it ran on the pdf page, outside of the extension.
By running inside the extension it works with local files
again.

BUG=574918, 520422
Committed: https://crrev.com/ceb0e8fda9c584873757779437948e929a95b3c9
Cr-Commit-Position: refs/heads/master@{#368172}

[dmazzoni, 2016-01-06 20:28:57]

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

[raymes, 2016-01-06 23:02:32]

Hey Dominic,

I think this would re-open a security hole fixed in
https://code.google.com/p/chromium/issues/detail?id=520422 . I tried to loop you
in on this issue at the time. Please have a read through that bug. Rob Wu stated
there was no regression with ChromeVox at the time but apparently that isn't
true. We need an alternative to make ChromeVox work. If it can run inside the
component extension, it should be able to directly call JS apis from there
without any cross-origin issues.

[dmazzoni, 2016-01-06 23:47:44]

Description was changed from

==========
Allow PDF to send accessibility reply messages to any origin.

This allows the ChromeVox content script to receive replies from the PDF
plug-in.

BUG=574918
==========

to

==========
Allow PDF to send accessibility reply messages to any origin.

This allows the ChromeVox content script to receive replies from the PDF
plug-in.

BUG=574918,520422
==========

[dmazzoni, 2016-01-06 23:50:07]

Yeah, I remember that bug but I trusted that ChromeVox would still work and
didn't check it manually. I guess it broke after all.

I think we can make it run inside the component extension. I tried that briefly
and it successfully sent the message to the plug-in, and the plug-in sent the
reply, but then pdf.js intercepted the message and posted it to the parent
window, which did nothing. ChromeVox never got it.

Should I modify pdf.js to re-post the json reply message to the same window? Or
have it ignore that message so ChromeVox can intercept it instead?

[raymes, 2016-01-06 23:59:38]

Ahh ok :) Before digging in, I'm confused why we see this working and you
don't!: https://code.google.com/p/chromium/issues/detail?id=574918

[raymes, 2016-01-07 02:29:22]

On 2016/01/06 23:50:07, dmazzoni wrote:
> Yeah, I remember that bug but I trusted that ChromeVox would still work and
> didn't check it manually. I guess it broke after all.
> 
> I think we can make it run inside the component extension. I tried that
briefly
> and it successfully sent the message to the plug-in, and the plug-in sent the
> reply, but then pdf.js intercepted the message and posted it to the parent
> window, which did nothing. ChromeVox never got it.
> 
> Should I modify pdf.js to re-post the json reply message to the same window?
Or
> have it ignore that message so ChromeVox can intercept it instead?

Cool - if you can run inside the component extension you should be able to do
the same thing. But instead of posting to the plugin (in chromevox), just post
to the current "window" and everything should just work. Let me know if that
works. We could do something a bit fancier with some changes in pdf.js but this
would be the most simple/backward compatible way.

[dmazzoni, 2016-01-07 20:41:06]

dmazzoni@chromium.org changed reviewers:
+ dtseng@chromium.org

[dmazzoni, 2016-01-07 20:41:07]

OK, fixed within ChromeVox now. David, could you review?

[dmazzoni, 2016-01-07 20:42:14]

Description was changed from

==========
Allow PDF to send accessibility reply messages to any origin.

This allows the ChromeVox content script to receive replies from the PDF
plug-in.

BUG=574918,520422
==========

to

==========
Run ChromeVox PDF extractor within Chrome's PDF extension

Previously it ran on the pdf page, outside of the extension.
By running inside the extension it works with local files
again.

BUG=574918,520422
==========

[dmazzoni, 2016-01-07 20:42:24]

Updated change description.

[David Tseng, 2016-01-07 21:28:05]

lgtm

https://codereview.chromium.org/1561883004/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/chromeos/chromevox/chromevox/injected/pdf_processor.js
(left):

https://codereview.chromium.org/1561883004/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/pdf_processor.js:13:
/**
Mention that this gets injected into the PDF plugin directly.

https://codereview.chromium.org/1561883004/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/chromeos/chromevox/chromevox/injected/pdf_processor.js
(right):

https://codereview.chromium.org/1561883004/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/pdf_processor.js:50:
'embed[type="application/x-google-chrome-pdf"]');
Does this change depend on specific versions of the PDF plugin/major Chrome
version?

https://codereview.chromium.org/1561883004/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/pdf_processor.js:82:
cvox.ChromeVox.navigationManager.reset();
How does this work if we're running in the PDF plugin's context?

[dmazzoni, 2016-01-07 21:41:41]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2016-01-07 21:41:41]

The patchset sent to the CQ was uploaded after l-g-t-m from dtseng@chromium.org
Link to the patchset: https://codereview.chromium.org/1561883004/#ps40001
(title: "Address feedback")

[commit-bot: I haz the power, 2016-01-07 21:41:58]

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

[commit-bot: I haz the power, 2016-01-07 22:13:30]

Description was changed from

==========
Run ChromeVox PDF extractor within Chrome's PDF extension

Previously it ran on the pdf page, outside of the extension.
By running inside the extension it works with local files
again.

BUG=574918,520422
==========

to

==========
Run ChromeVox PDF extractor within Chrome's PDF extension

Previously it ran on the pdf page, outside of the extension.
By running inside the extension it works with local files
again.

BUG=574918,520422
==========

[commit-bot: I haz the power, 2016-01-07 22:13:31]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-01-07 22:14:16]

Description was changed from

==========
Run ChromeVox PDF extractor within Chrome's PDF extension

Previously it ran on the pdf page, outside of the extension.
By running inside the extension it works with local files
again.

BUG=574918,520422
==========

to

==========
Run ChromeVox PDF extractor within Chrome's PDF extension

Previously it ran on the pdf page, outside of the extension.
By running inside the extension it works with local files
again.

BUG=574918,520422

Committed: https://crrev.com/ceb0e8fda9c584873757779437948e929a95b3c9
Cr-Commit-Position: refs/heads/master@{#368172}
==========

[commit-bot: I haz the power, 2016-01-07 22:14:17]

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

[dmazzoni, 2016-01-08 04:41:18]

https://codereview.chromium.org/1561883004/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/chromeos/chromevox/chromevox/injected/pdf_processor.js
(left):

https://codereview.chromium.org/1561883004/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/pdf_processor.js:13:
/**
On 2016/01/07 21:28:05, David Tseng wrote:
> Mention that this gets injected into the PDF plugin directly.

Done, though to clarify, it gets injected into the extension, which wraps the
plug-in. The extension itself is wrapped by another page which is where we used
to run this code.

https://codereview.chromium.org/1561883004/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/chromeos/chromevox/chromevox/injected/pdf_processor.js
(right):

https://codereview.chromium.org/1561883004/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/pdf_processor.js:50:
'embed[type="application/x-google-chrome-pdf"]');
On 2016/01/07 21:28:05, David Tseng wrote:
> Does this change depend on specific versions of the PDF plugin/major Chrome
> version?

Not that I know of, this has been stable for a while.

https://codereview.chromium.org/1561883004/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/pdf_processor.js:82:
cvox.ChromeVox.navigationManager.reset();
On 2016/01/07 21:28:05, David Tseng wrote:
> How does this work if we're running in the PDF plugin's context?

It's the context of the extension.