PDF: Allow external navigation links in PDF bookmarks

PDF: Allow external navigation links in PDF bookmarks

Adds support for table of contents entries which point to external URIs.
These bookmarks will open the link in a new tab when clicked.

BUG=581290
Committed: https://crrev.com/1e42ed1a044a97823e2beed66cfcf71f42c5a2c2
Cr-Commit-Position: refs/heads/master@{#372862}

[tsergeant, 2016-02-01 02:51:23]

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

[tsergeant, 2016-02-01 02:51:23]

Raymes, please take a look!

[raymes, 2016-02-01 04:44:32]

https://codereview.chromium.org/1654683002/diff/1/chrome/browser/resources/pd...
File chrome/browser/resources/pdf/elements/viewer-bookmark/viewer-bookmark.js
(right):

https://codereview.chromium.org/1654683002/diff/1/chrome/browser/resources/pd...
chrome/browser/resources/pdf/elements/viewer-bookmark/viewer-bookmark.js:69:
this.fire('navigate', {uri: this.bookmark.uri, newtab: true});
perhaps just reuse the change-page event?

https://codereview.chromium.org/1654683002/diff/1/pdf/pdfium/pdfium_engine.cc
File pdf/pdfium/pdfium_engine.cc (right):

https://codereview.chromium.org/1654683002/diff/1/pdf/pdfium/pdfium_engine.cc...
pdf/pdfium/pdfium_engine.cc:433: buffer_size = FPDFAction_GetURIPath(doc,
action, NULL, 0);
nit: nullptr

https://codereview.chromium.org/1654683002/diff/1/pdf/pdfium/pdfium_engine.cc...
pdf/pdfium/pdfium_engine.cc:434: std::string uri;
nit: move into if statement

[tsergeant, 2016-02-01 06:01:00]

https://codereview.chromium.org/1654683002/diff/1/chrome/browser/resources/pd...
File chrome/browser/resources/pdf/elements/viewer-bookmark/viewer-bookmark.js
(right):

https://codereview.chromium.org/1654683002/diff/1/chrome/browser/resources/pd...
chrome/browser/resources/pdf/elements/viewer-bookmark/viewer-bookmark.js:69:
this.fire('navigate', {uri: this.bookmark.uri, newtab: true});
On 2016/02/01 04:44:32, raymes wrote:
> perhaps just reuse the change-page event?

I'm not convinced that this is an improvement. They're fairly different things
(`change-page` isn't a good name to cover both actions), and they get routed to
different places in pdf.js (navigator vs viewport).

https://codereview.chromium.org/1654683002/diff/1/pdf/pdfium/pdfium_engine.cc
File pdf/pdfium/pdfium_engine.cc (right):

https://codereview.chromium.org/1654683002/diff/1/pdf/pdfium/pdfium_engine.cc...
pdf/pdfium/pdfium_engine.cc:433: buffer_size = FPDFAction_GetURIPath(doc,
action, NULL, 0);
On 2016/02/01 04:44:32, raymes wrote:
> nit: nullptr

Done (and the one above)

https://codereview.chromium.org/1654683002/diff/1/pdf/pdfium/pdfium_engine.cc...
pdf/pdfium/pdfium_engine.cc:434: std::string uri;
On 2016/02/01 04:44:32, raymes wrote:
> nit: move into if statement 

Done.

[tsergeant, 2016-02-01 22:41:40]

Oops, I didn't upload properly. The new patchset is actually there now.

[raymes, 2016-02-02 00:20:14]

lgtm

https://codereview.chromium.org/1654683002/diff/1/chrome/browser/resources/pd...
File chrome/browser/resources/pdf/elements/viewer-bookmark/viewer-bookmark.js
(right):

https://codereview.chromium.org/1654683002/diff/1/chrome/browser/resources/pd...
chrome/browser/resources/pdf/elements/viewer-bookmark/viewer-bookmark.js:69:
this.fire('navigate', {uri: this.bookmark.uri, newtab: true});
Ah I assumed that change-page was only used here, so we could change it into a
"bookmark-clicked" type event. But that's not the case. This seems ok to me.

[tsergeant, 2016-02-02 00:32:48]

The CQ bit was checked by tsergeant@chromium.org

[commit-bot: I haz the power, 2016-02-02 00:38:38]

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

[commit-bot: I haz the power, 2016-02-02 02:14:17]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-02-02 02:22:07]

Description was changed from

==========
PDF: Allow external navigation links in PDF bookmarks

Adds support for table of contents entries which point to external URIs.
These bookmarks will open the link in a new tab when clicked.

BUG=581290
==========

to

==========
PDF: Allow external navigation links in PDF bookmarks

Adds support for table of contents entries which point to external URIs.
These bookmarks will open the link in a new tab when clicked.

BUG=581290

Committed: https://crrev.com/1e42ed1a044a97823e2beed66cfcf71f42c5a2c2
Cr-Commit-Position: refs/heads/master@{#372862}
==========

[commit-bot: I haz the power, 2016-02-02 02:22:08]

Patchset 2 (id:??) landed as
https://crrev.com/1e42ed1a044a97823e2beed66cfcf71f42c5a2c2
Cr-Commit-Position: refs/heads/master@{#372862}