Testcases for nameddests and navigate for PDF

Testcases for nameddests for PDF.

Test cases added for nameddests and navigate.
common_test.js file have been added that have the common function for viewport_test and nameddestinations_test file.

This is followup for 447888 issue.

BUG=450105
Committed: https://crrev.com/dc24a5d7603a57d8da13c18c6e7adf114f373a85
Cr-Commit-Position: refs/heads/master@{#314119}

[Deepak, 2015-01-20 05:48:31]

deepak.m1@samsung.com changed reviewers:
+ raymes@chromium.org

[Deepak, 2015-01-20 05:48:32]

Dear Raymes,
PTAL at my changes.
Thanks.

[Deepak, 2015-01-21 06:25:34]

Dear Raymes,
 PTAL at my changes and share your thoughts.
Thanks.

[raymes, 2015-01-23 00:01:44]

https://codereview.chromium.org/838723003/diff/1/chrome/browser/resources/pdf...
File chrome/browser/resources/pdf/pdf_extension_test.cc (right):

https://codereview.chromium.org/838723003/diff/1/chrome/browser/resources/pdf...
chrome/browser/resources/pdf/pdf_extension_test.cc:100:
RunTestsInFile("nameddestinations_test.js", "test_nameddestinations.pdf");
Since we're not actually using the PDF involved, we can just pass in "test.pdf"

https://codereview.chromium.org/838723003/diff/1/chrome/test/data/pdf/namedde...
File chrome/test/data/pdf/nameddestinations_test.js (right):

https://codereview.chromium.org/838723003/diff/1/chrome/test/data/pdf/namedde...
chrome/test/data/pdf/nameddestinations_test.js:9: function
testNamedDestinations() {
I see you've mostly tested the params parser here, but we should do it using the
public API. We can create a new OpenPDFParamsParser just for the test and then
run a bunch of URLs through it to test it, rather than accessing the one inside
viewer.paramsParser_. Does that make sense?

https://codereview.chromium.org/838723003/diff/1/chrome/test/data/pdf/namedde...
chrome/test/data/pdf/nameddestinations_test.js:73: ];
Can we also add a test for the Navigator class? We can create a new Navigator
object in the test and pass in a new paramsParser and a mock viewport. We can
also pass in a mock  window object to the Navigator. Then we can call navigate()
with a bunch of different URLs and then check that the mock viewport object gets
navigated to the right places /or/ the mock window gets navigated to the right
places. To see what I mean by a "mock" class, you can look inside
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/test/data/p...

[Deepak, 2015-01-23 14:05:15]

@Raymes
Thanks for review.
PTAL

https://codereview.chromium.org/838723003/diff/1/chrome/browser/resources/pdf...
File chrome/browser/resources/pdf/pdf_extension_test.cc (right):

https://codereview.chromium.org/838723003/diff/1/chrome/browser/resources/pdf...
chrome/browser/resources/pdf/pdf_extension_test.cc:100:
RunTestsInFile("nameddestinations_test.js", "test_nameddestinations.pdf");
On 2015/01/23 00:01:44, raymes wrote:
> Since we're not actually using the PDF involved, we can just pass in
"test.pdf"

Done.

https://codereview.chromium.org/838723003/diff/1/chrome/test/data/pdf/namedde...
File chrome/test/data/pdf/nameddestinations_test.js (right):

https://codereview.chromium.org/838723003/diff/1/chrome/test/data/pdf/namedde...
chrome/test/data/pdf/nameddestinations_test.js:9: function
testNamedDestinations() {
On 2015/01/23 00:01:44, raymes wrote:
> I see you've mostly tested the params parser here, but we should do it using
the
> public API. We can create a new OpenPDFParamsParser just for the test and then
> run a bunch of URLs through it to test it, rather than accessing the one
inside
> viewer.paramsParser_. Does that make sense?
> 

Yes, It make sense. I understood.

https://codereview.chromium.org/838723003/diff/1/chrome/test/data/pdf/namedde...
chrome/test/data/pdf/nameddestinations_test.js:73: ];
On 2015/01/23 00:01:44, raymes wrote:
> Can we also add a test for the Navigator class? We can create a new Navigator
> object in the test and pass in a new paramsParser and a mock viewport. We can
> also pass in a mock  window object to the Navigator. Then we can call
navigate()
> with a bunch of different URLs and then check that the mock viewport object
gets
> navigated to the right places /or/ the mock window gets navigated to the right
> places. To see what I mean by a "mock" class, you can look inside
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/test/data/p...

yes,I understand your direction. I have checked it.
Thanks , This will make my understanding better about test case.
If you allow, Can I do this in a separate CL ?

[Deepak, 2015-01-27 03:32:31]

PTAL.

[Deepak, 2015-01-27 05:14:30]

deepak.m1@samsung.com changed reviewers:
+ gene@chromium.org, thestig@chromium.org

[raymes, 2015-01-28 01:33:45]

Thanks!

https://codereview.chromium.org/838723003/diff/80001/chrome/browser/resources...
File chrome/browser/resources/pdf/pdf_extension_test.cc (right):

https://codereview.chromium.org/838723003/diff/80001/chrome/browser/resources...
chrome/browser/resources/pdf/pdf_extension_test.cc:45: std::string commonfile) {
Let's not pass this as a param. Instead let's just always load "test_util.js"
for every test that gets run.

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/com...
File chrome/test/data/pdf/common_file.js (right):

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/com...
chrome/test/data/pdf/common_file.js:1: // Copyright 2015 The Chromium Authors.
All rights reserved.
Can we rename this file test_util.js

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/com...
chrome/test/data/pdf/common_file.js:5: // It have common functions used in
Viewport test and NamedDestinations test.
// Utilities that are used in multiple tests.

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/nam...
File chrome/test/data/pdf/nameddestinations_test.js (right):

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/nam...
chrome/test/data/pdf/nameddestinations_test.js:1: // Copyright 2015 The Chromium
Authors. All rights reserved.
We could really split this into 2 files:
params_parser_test.js
navigator_test.js

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/nam...
chrome/test/data/pdf/nameddestinations_test.js:9: function
testNamedDestinationsByNavigate() {
nit: testNavigate

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/nam...
chrome/test/data/pdf/nameddestinations_test.js:20: var navigator = new
Navigator(url, viewport, paramsParser);
Can we change Navigator to also pass in:
-A function to open a new tab
-A function to navigate the current tab

Then we could test that these functions get called

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/nam...
chrome/test/data/pdf/nameddestinations_test.js:23: 
nit: no need for a blank line

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/nam...
chrome/test/data/pdf/nameddestinations_test.js:38: // This shouldl open
"http://xyz.pdf#US" in newTab. So current tab viewport
nit: shouldl->should
nit: newTab->a new tab

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/nam...
chrome/test/data/pdf/nameddestinations_test.js:53: // #ABC is not a
nemeddestination in the page so viewport should not change
nit: nemeddestination->named destination

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/nam...
chrome/test/data/pdf/nameddestinations_test.js:66: function
testNamedDestinations() {
nit: testParamsParser

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/nam...
chrome/test/data/pdf/nameddestinations_test.js:94: "#zoom=200,100,200");
nit: indentation. Use 4 space indent

[raymes, 2015-01-28 01:48:47]

Thanks!

[Deepak, 2015-01-28 05:57:24]

Thanks for review. Changes updated.
PTAL.

https://codereview.chromium.org/838723003/diff/80001/chrome/browser/resources...
File chrome/browser/resources/pdf/pdf_extension_test.cc (right):

https://codereview.chromium.org/838723003/diff/80001/chrome/browser/resources...
chrome/browser/resources/pdf/pdf_extension_test.cc:45: std::string commonfile) {
On 2015/01/28 01:33:45, raymes wrote:
> Let's not pass this as a param. Instead let's just always load "test_util.js"
> for every test that gets run.

Done.

https://codereview.chromium.org/838723003/diff/80001/chrome/browser/resources...
chrome/browser/resources/pdf/pdf_extension_test.cc:45: std::string commonfile) {
On 2015/01/28 01:33:45, raymes wrote:
> Let's not pass this as a param. Instead let's just always load "test_util.js"
> for every test that gets run.

Done.

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/com...
File chrome/test/data/pdf/common_file.js (right):

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/com...
chrome/test/data/pdf/common_file.js:1: // Copyright 2015 The Chromium Authors.
All rights reserved.
On 2015/01/28 01:33:45, raymes wrote:
> Can we rename this file test_util.js

Done.

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/com...
chrome/test/data/pdf/common_file.js:5: // It have common functions used in
Viewport test and NamedDestinations test.
On 2015/01/28 01:33:45, raymes wrote:
> // Utilities that are used in multiple tests.

Done.

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/nam...
File chrome/test/data/pdf/nameddestinations_test.js (right):

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/nam...
chrome/test/data/pdf/nameddestinations_test.js:1: // Copyright 2015 The Chromium
Authors. All rights reserved.
On 2015/01/28 01:33:45, raymes wrote:
> We could really split this into 2 files:
> params_parser_test.js
> navigator_test.js

Done.

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/nam...
chrome/test/data/pdf/nameddestinations_test.js:9: function
testNamedDestinationsByNavigate() {
On 2015/01/28 01:33:45, raymes wrote:
> nit: testNavigate

Done.

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/nam...
chrome/test/data/pdf/nameddestinations_test.js:20: var navigator = new
Navigator(url, viewport, paramsParser);
On 2015/01/28 01:33:45, raymes wrote:
> Can we change Navigator to also pass in:
> -A function to open a new tab
> -A function to navigate the current tab
> 
> Then we could test that these functions get called

Done.

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/nam...
chrome/test/data/pdf/nameddestinations_test.js:23: 
On 2015/01/28 01:33:45, raymes wrote:
> nit: no need for a blank line

Done.

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/nam...
chrome/test/data/pdf/nameddestinations_test.js:38: // This shouldl open
"http://xyz.pdf#US" in newTab. So current tab viewport
On 2015/01/28 01:33:45, raymes wrote:
> nit: shouldl->should
> nit: newTab->a new tab

Done.

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/nam...
chrome/test/data/pdf/nameddestinations_test.js:53: // #ABC is not a
nemeddestination in the page so viewport should not change
On 2015/01/28 01:33:45, raymes wrote:
> nit: nemeddestination->named destination

Done.

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/nam...
chrome/test/data/pdf/nameddestinations_test.js:66: function
testNamedDestinations() {
On 2015/01/28 01:33:45, raymes wrote:
> nit: testParamsParser

Done.

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/nam...
chrome/test/data/pdf/nameddestinations_test.js:66: function
testNamedDestinations() {
On 2015/01/28 01:33:45, raymes wrote:
> nit: testParamsParser

Done.

https://codereview.chromium.org/838723003/diff/80001/chrome/test/data/pdf/nam...
chrome/test/data/pdf/nameddestinations_test.js:94: "#zoom=200,100,200");
On 2015/01/28 01:33:45, raymes wrote:
> nit: indentation. Use 4 space indent

Done.

[raymes, 2015-01-29 04:27:14]

Looks good! just a few small things

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

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/navigator.js:12: * @param {Object}
navigateInCurrentTabCallback The Callback function that get
nit: get->gets
nit: {Object}->{Function}

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/navigator.js:13: *    called when navigation happen
in the current tab.
nit: happen->happens

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/navigator.js:14: * @param {Object}
navigateInNewTabCallback The Callback function that get
nit: get->gets
nit: {Object}->{Function}

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/navigator.js:15: *    called when navigation happen
in the new tab.
nit: happen->happens

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/navigator.js:18: viewport,
nit: indentation - please align with the (

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/navigator.js:81: window.open(url);
We should remove the above 4 lines (and comment) now and pass them in as the
navigateInNewTabCallback function

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/navigator.js:89: window.location.href = url;
The same with this line - it should be passed in as the
navigateInCurrentTabCallback

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
File chrome/browser/resources/pdf/pdf_extension_test.cc (right):

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf_extension_test.cc:76: 
nit: no need to add a newline

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf_extension_test.cc:77: base::FilePath
test_file_data_dir = test_data_dir.AppendASCII(filename);
this can be called "test_file_path"

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf_extension_test.cc:82: std::string
test_common_js;
test_util_js

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf_extension_test.cc:83: test_data_dir =
test_data_dir.AppendASCII("test_util.js");
Create a separate base::FilePath here called "test_util_path

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf_extension_test.cc:85:
test_js.append(test_common_js);
I would append the test_common_js before the test_js

[Deepak, 2015-01-29 06:05:41]

Thanks for review.
All suggestions addressed.
PTAL.

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

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/navigator.js:12: * @param {Object}
navigateInCurrentTabCallback The Callback function that get
On 2015/01/29 04:27:13, raymes wrote:
> nit: get->gets
> nit: {Object}->{Function}

Done.

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/navigator.js:13: *    called when navigation happen
in the current tab.
On 2015/01/29 04:27:13, raymes wrote:
> nit: happen->happens

Done.

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/navigator.js:14: * @param {Object}
navigateInNewTabCallback The Callback function that get
On 2015/01/29 04:27:13, raymes wrote:
> nit: get->gets
> nit: {Object}->{Function}

Done.

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/navigator.js:15: *    called when navigation happen
in the new tab.
On 2015/01/29 04:27:13, raymes wrote:
> nit: happen->happens

Done.

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/navigator.js:18: viewport,
On 2015/01/29 04:27:13, raymes wrote:
> nit: indentation - please align with the (

Done.

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/navigator.js:81: window.open(url);
On 2015/01/29 04:27:13, raymes wrote:
> We should remove the above 4 lines (and comment) now and pass them in as the
> navigateInNewTabCallback function

Done.

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/navigator.js:89: window.location.href = url;
On 2015/01/29 04:27:13, raymes wrote:
> The same with this line - it should be passed in as the
> navigateInCurrentTabCallback

Done.

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
File chrome/browser/resources/pdf/pdf_extension_test.cc (right):

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf_extension_test.cc:76: 
On 2015/01/29 04:27:13, raymes wrote:
> nit: no need to add a newline

Done.

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf_extension_test.cc:77: base::FilePath
test_file_data_dir = test_data_dir.AppendASCII(filename);
On 2015/01/29 04:27:13, raymes wrote:
> this can be called "test_file_path"

Done.

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf_extension_test.cc:82: std::string
test_common_js;
On 2015/01/29 04:27:13, raymes wrote:
> test_util_js

Done.

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf_extension_test.cc:83: test_data_dir =
test_data_dir.AppendASCII("test_util.js");
On 2015/01/29 04:27:13, raymes wrote:
> Create a separate base::FilePath here called "test_util_path

Done.

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf_extension_test.cc:85:
test_js.append(test_common_js);
On 2015/01/29 04:27:13, raymes wrote:
> I would append the test_common_js before the test_js

Done.

[raymes, 2015-01-30 03:45:04]

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

https://codereview.chromium.org/838723003/diff/140001/chrome/browser/resource...
chrome/browser/resources/pdf/navigator.js:76: // window.open doesn't have this
guarantee.
Please keep this comment in the function that we pull out

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

https://codereview.chromium.org/838723003/diff/180001/chrome/browser/resource...
chrome/browser/resources/pdf/navigator.js:46: navigateInNewTab_: function(url) {
These functions should be defined inside pdf.js and they should be passed into
navigator.

https://codereview.chromium.org/838723003/diff/180001/chrome/browser/resource...
chrome/browser/resources/pdf/navigator.js:99: this.navigateInNewTab_(url);
this should just call this.navigateInNewTabCallback_();

https://codereview.chromium.org/838723003/diff/180001/chrome/browser/resource...
chrome/browser/resources/pdf/navigator.js:106: this.navigateInCurrentTab_(url);
this should just call this.navigateInCurrentTabCallback_();

https://codereview.chromium.org/838723003/diff/180001/chrome/test/data/pdf/na...
File chrome/test/data/pdf/navigator_test.js (right):

https://codereview.chromium.org/838723003/diff/180001/chrome/test/data/pdf/na...
chrome/test/data/pdf/navigator_test.js:27: * Test navigation with in the page,
opening url in the same tab and
nit: with in->within
nit: opening a url

[Deepak, 2015-01-30 04:39:21]

Thanks for Review.
PTAL

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

https://codereview.chromium.org/838723003/diff/180001/chrome/browser/resource...
chrome/browser/resources/pdf/navigator.js:46: navigateInNewTab_: function(url) {
On 2015/01/30 03:45:03, raymes wrote:
> These functions should be defined inside pdf.js and they should be passed into
> navigator.

Done.

https://codereview.chromium.org/838723003/diff/180001/chrome/browser/resource...
chrome/browser/resources/pdf/navigator.js:99: this.navigateInNewTab_(url);
On 2015/01/30 03:45:03, raymes wrote:
> this should just call this.navigateInNewTabCallback_();

Done.

https://codereview.chromium.org/838723003/diff/180001/chrome/browser/resource...
chrome/browser/resources/pdf/navigator.js:106: this.navigateInCurrentTab_(url);
On 2015/01/30 03:45:03, raymes wrote:
> this should just call this.navigateInCurrentTabCallback_();

Done.

https://codereview.chromium.org/838723003/diff/180001/chrome/test/data/pdf/na...
File chrome/test/data/pdf/navigator_test.js (right):

https://codereview.chromium.org/838723003/diff/180001/chrome/test/data/pdf/na...
chrome/test/data/pdf/navigator_test.js:27: * Test navigation with in the page,
opening url in the same tab and
On 2015/01/30 03:45:03, raymes wrote:
> nit: with in->within
> nit: opening a url

Done.

[Deepak, 2015-02-01 04:10:49]

@Raymes
These changes are ready to rolled in.
Thanks
PTAL

[raymes, 2015-02-01 23:50:38]

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

https://codereview.chromium.org/838723003/diff/200001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf.js:380: },
These don't need to be functions hanging off of PDFViewer. You can define them
outside of the class.

https://codereview.chromium.org/838723003/diff/200001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf.js:388: if (chrome.tabs)
We used to have a comment here, it disapeared!

https://codereview.chromium.org/838723003/diff/200001/chrome/browser/resource...
File chrome/browser/resources/pdf/pdf_extension_test.cc (right):

https://codereview.chromium.org/838723003/diff/200001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf_extension_test.cc:112:
IN_PROC_BROWSER_TEST_F(PDFExtensionTest, NamedDestionations) {
nit: NamedDestionations->ParamsParser

[Deepak, 2015-02-02 04:00:12]

Thanks for review.Changes done as suggested.
PTAL
Thanks.

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

https://codereview.chromium.org/838723003/diff/200001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf.js:380: },
On 2015/02/01 23:50:38, raymes wrote:
> These don't need to be functions hanging off of PDFViewer. You can define them
> outside of the class.

Done.

https://codereview.chromium.org/838723003/diff/200001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf.js:388: if (chrome.tabs)
On 2015/02/01 23:50:38, raymes wrote:
> We used to have a comment here, it disapeared!

Done.

https://codereview.chromium.org/838723003/diff/200001/chrome/browser/resource...
File chrome/browser/resources/pdf/pdf_extension_test.cc (right):

https://codereview.chromium.org/838723003/diff/200001/chrome/browser/resource...
chrome/browser/resources/pdf/pdf_extension_test.cc:112:
IN_PROC_BROWSER_TEST_F(PDFExtensionTest, NamedDestionations) {
On 2015/02/01 23:50:38, raymes wrote:
> nit: NamedDestionations->ParamsParser

Done.

[raymes, 2015-02-02 04:04:43]

lgtm

[Deepak, 2015-02-02 04:05:31]

On 2015/02/02 04:04:43, raymes wrote:
> lgtm

Thanks.

[Deepak, 2015-02-02 04:05:40]

The CQ bit was checked by deepak.m1@samsung.com

[commit-bot: I haz the power, 2015-02-02 04:06:07]

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

[commit-bot: I haz the power, 2015-02-02 06:59:57]

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

[commit-bot: I haz the power, 2015-02-02 06:59:58]

Try jobs failed on following builders:
  win8_chromium_gn_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_gn_...)

[Deepak, 2015-02-02 07:07:24]

The CQ bit was checked by deepak.m1@samsung.com

[commit-bot: I haz the power, 2015-02-02 07:07:47]

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

[commit-bot: I haz the power, 2015-02-02 09:27:11]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2015-02-02 09:28:06]

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