Modify pdf_scripting_api.js to support postMessage using BrowserPlugin

Modify pdf_scripting_api.js to support postMessage using BrowserPlugin

This modifies how pdf_scripting_api.js interacts with the PDF extension. Previously messages would just be posted to the parent window by pdf.js. But inside BrowserPlugin there isn't a notion of a parent window. Instead, the embedder can post a message into the plugin and pdf.js uses the source window of that message to respond.

A protocol is setup where pdf_scripting_api.js sends a 'setParentWindow' message to initialize the channel. pdf.js responds with a 'readyToReceieve' message when the plugin is initialized, at which point further messages can be safely sent. Since the 'setParentWindow' message can be received by pdf.js at any point after the page has loaded, the code has been changed a little to install the message handler on page load rather than when the stream is available (an init function has been added to PDFViewer to do initialization when the stream has loaded).

There are a few other more cosmetic fixes to the code here as well.

BUG=415858
Committed: https://crrev.com/850e893700850b70959f6d5baf879e9b5916f28b
Cr-Commit-Position: refs/heads/master@{#310632}

[raymes, 2015-01-06 03:16:27]

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

[Sam McNally, 2015-01-06 05:31:04]

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

https://codereview.chromium.org/823813002/diff/120001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf.js:45: this.documentLoaded_ = false;
The public "loaded" property is still used by the test.

https://codereview.chromium.org/823813002/diff/120001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf.js:71: init: function(streamDetails) {
How about setStreamDetails?

https://codereview.chromium.org/823813002/diff/120001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf.js:605: if (this.parentWindow_)
Won't this lose messages?

https://codereview.chromium.org/823813002/diff/120001/chrome/browser/resource...
File chrome/browser/resources/pdf/pdf_scripting_api.js (right):

https://codereview.chromium.org/823813002/diff/120001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf_scripting_api.js:9: * @param {string} plugin
the plugin element containing the pdf viewer.
Not a string anymore.

https://codereview.chromium.org/823813002/diff/120001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf_scripting_api.js:174:
'chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/index.html?' + src);
Either line this up with the ( above or break the previous line before 'src'.

https://codereview.chromium.org/823813002/diff/120001/chrome/test/data/pdf/ba...
File chrome/test/data/pdf/basic_plugin_test.js (right):

https://codereview.chromium.org/823813002/diff/120001/chrome/test/data/pdf/ba...
chrome/test/data/pdf/basic_plugin_test.js:24: var client = new
PDFScriptingAPI(window, window, window.location.origin);
Remove window.location.origin.

https://codereview.chromium.org/823813002/diff/120001/chrome/test/data/pdf/ba...
chrome/test/data/pdf/basic_plugin_test.js:34: var client = new
PDFScriptingAPI(window, window, window.location.origin);
Same here.

https://codereview.chromium.org/823813002/diff/120001/chrome/test/data/pdf/ba...
chrome/test/data/pdf/basic_plugin_test.js:54: var client = new
PDFScriptingAPI(window, window, window.location.origin);
And here.

[raymes, 2015-01-07 03:17:58]

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

https://codereview.chromium.org/823813002/diff/120001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf.js:45: this.documentLoaded_ = false;
I removed the use of it.

https://codereview.chromium.org/823813002/diff/120001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf.js:71: init: function(streamDetails) {
I ended up not needing the init function so I've gone back to the previous
design.

https://codereview.chromium.org/823813002/diff/120001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf.js:605: if (this.parentWindow_)
On 2015/01/06 05:31:03, Sam McNally wrote:
> Won't this lose messages?

Yes, but that's ok. I've made it always send a document loaded message on
setParentWindow if the document has loaded.

https://codereview.chromium.org/823813002/diff/120001/chrome/browser/resource...
File chrome/browser/resources/pdf/pdf_scripting_api.js (right):

https://codereview.chromium.org/823813002/diff/120001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf_scripting_api.js:9: * @param {string} plugin
the plugin element containing the pdf viewer.
On 2015/01/06 05:31:04, Sam McNally wrote:
> Not a string anymore.

Done.

https://codereview.chromium.org/823813002/diff/120001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf_scripting_api.js:174:
'chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/index.html?' + src);
On 2015/01/06 05:31:04, Sam McNally wrote:
> Either line this up with the ( above or break the previous line before 'src'.

Done.

https://codereview.chromium.org/823813002/diff/120001/chrome/test/data/pdf/ba...
File chrome/test/data/pdf/basic_plugin_test.js (right):

https://codereview.chromium.org/823813002/diff/120001/chrome/test/data/pdf/ba...
chrome/test/data/pdf/basic_plugin_test.js:24: var client = new
PDFScriptingAPI(window, window, window.location.origin);
On 2015/01/06 05:31:04, Sam McNally wrote:
> Remove window.location.origin.

Done.

https://codereview.chromium.org/823813002/diff/120001/chrome/test/data/pdf/ba...
chrome/test/data/pdf/basic_plugin_test.js:34: var client = new
PDFScriptingAPI(window, window, window.location.origin);
On 2015/01/06 05:31:04, Sam McNally wrote:
> Same here.

Done.

https://codereview.chromium.org/823813002/diff/120001/chrome/test/data/pdf/ba...
chrome/test/data/pdf/basic_plugin_test.js:54: var client = new
PDFScriptingAPI(window, window, window.location.origin);
On 2015/01/06 05:31:04, Sam McNally wrote:
> And here.

Done.

[Sam McNally, 2015-01-07 05:01:04]

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

https://codereview.chromium.org/823813002/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf.js:575: if (this.documentLoaded_) {
loaded_

[raymes, 2015-01-07 06:10:39]

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

https://codereview.chromium.org/823813002/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf.js:575: if (this.documentLoaded_) {
On 2015/01/07 05:01:04, Sam McNally wrote:
> loaded_

Done.

[Sam McNally, 2015-01-08 03:50:26]

LGTM

https://codereview.chromium.org/823813002/diff/180001/chrome/browser/resource...
File chrome/browser/resources/pdf/main.js (right):

https://codereview.chromium.org/823813002/diff/180001/chrome/browser/resource...
chrome/browser/resources/pdf/main.js:64: function(streamDetails) {
initViewer(streamDetails); });
Pass initViewer directly.

https://codereview.chromium.org/823813002/diff/180001/chrome/browser/resource...
chrome/browser/resources/pdf/main.js:92: main();
How about doing the (function() {})() thing here too?

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

https://codereview.chromium.org/823813002/diff/180001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf.js:526: console.error(message);
Remove.

https://codereview.chromium.org/823813002/diff/180001/chrome/test/data/pdf/ba...
File chrome/test/data/pdf/basic_plugin_test.js (right):

https://codereview.chromium.org/823813002/diff/180001/chrome/test/data/pdf/ba...
chrome/test/data/pdf/basic_plugin_test.js:24: var client = new
PDFScriptingAPI(window, window);
Can the tests reuse the client created before starting the tests?

[raymes, 2015-01-08 22:08:21]

https://codereview.chromium.org/823813002/diff/180001/chrome/browser/resource...
File chrome/browser/resources/pdf/main.js (right):

https://codereview.chromium.org/823813002/diff/180001/chrome/browser/resource...
chrome/browser/resources/pdf/main.js:64: function(streamDetails) {
initViewer(streamDetails); });
On 2015/01/08 03:50:26, Sam McNally wrote:
> Pass initViewer directly.

Done.

https://codereview.chromium.org/823813002/diff/180001/chrome/browser/resource...
chrome/browser/resources/pdf/main.js:92: main();
I almost did but it added more brackets everywhere. I thought it looked less
cluttered/clearer just to call main.

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

https://codereview.chromium.org/823813002/diff/180001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf.js:526: console.error(message);
On 2015/01/08 03:50:26, Sam McNally wrote:
> Remove.

Done.

https://codereview.chromium.org/823813002/diff/180001/chrome/test/data/pdf/ba...
File chrome/test/data/pdf/basic_plugin_test.js (right):

https://codereview.chromium.org/823813002/diff/180001/chrome/test/data/pdf/ba...
chrome/test/data/pdf/basic_plugin_test.js:24: var client = new
PDFScriptingAPI(window, window);
On 2015/01/08 03:50:26, Sam McNally wrote:
> Can the tests reuse the client created before starting the tests?

Done.

[raymes, 2015-01-08 22:08:57]

The CQ bit was checked by raymes@chromium.org

[commit-bot: I haz the power, 2015-01-08 22:09:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/823813002/200001

[commit-bot: I haz the power, 2015-01-08 23:52:23]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2015-01-08 23:53:42]

Patchset 11 (id:??) landed as
https://crrev.com/850e893700850b70959f6d5baf879e9b5916f28b
Cr-Commit-Position: refs/heads/master@{#310632}