[Reland 1] Save pdf file instead of web page through context menu for embedded pdf file

Remove the flaky test as the flaky test is not really necessary. 

>Save pdf file instead of web page through context menu for embedded pdf file

>When user clicks 'Save as...' in the context menu over an embedded pdf file,
>we should save the pdf file instead of the entire web page. We fix this by showing
>the correct context menu for embedded pdf versus full page pdf: only full page
>pdf's context menu shows page navigation items such as 'Reload',
>but embedded pdf's context menu doesn't show them.   

>BUG=chromium:591071
>Review-Url: https://codereview.chromium.org/2862583010
>Cr-Commit-Position: refs/heads/master@{#473105}
>Committed: >https://chromium.googlesource.com/chromium/src/+/fd0e534843ac5926c1b26c7e1e7a7229fcb3aeee

Review-Url: https://codereview.chromium.org/2862583010
Cr-Commit-Position: refs/heads/master@{#473625}
Committed: https://chromium.googlesource.com/chromium/src/+/85a3174dfc2310377872a88db75f7370fa2f9734

[Wei Li, 2017-05-05 23:03:16]

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

[commit-bot: I haz the power, 2017-05-05 23:05:15]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Wei Li, 2017-05-05 23:17:45]

Description was changed from

==========
Save pdf file instead of web page through context menu for embedded pdf file

When user clicks 'Save as...' in the context menu over an embedded pdf file,
we should save the pdf file instead of the entire web page. We fix this by
showing
the correct context menu for embedded pdf versus full page pdf.

BUG=chromium:591071
==========

to

==========
Save pdf file instead of web page through context menu for embedded pdf file

When user clicks 'Save as...' in the context menu over an embedded pdf file,
we should save the pdf file instead of the entire web page. We fix this by
showing
the correct context menu for embedded pdf versus full page pdf: only full page
pdf's context menu shows page navigation items such as 'Back' and 'Forward',
but embedded pdf's context menu doesn't show them.   

BUG=chromium:591071
==========

[commit-bot: I haz the power, 2017-05-05 23:51:45]

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

[commit-bot: I haz the power, 2017-05-05 23:51:46]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Wei Li, 2017-05-06 03:49:51]

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

[commit-bot: I haz the power, 2017-05-06 03:50:24]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-06 04:44:52]

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

[commit-bot: I haz the power, 2017-05-06 04:44:52]

Dry run: This issue passed the CQ dry run.

[Wei Li, 2017-05-06 18:42:06]

weili@chromium.org changed reviewers:
+ lazyboy@chromium.org

[Wei Li, 2017-05-06 18:42:06]

ptal, thanks

[Wei Li, 2017-05-11 03:41:39]

ping?

[lazyboy, 2017-05-11 18:15:17]

Thanks for fixing, some comments below.

As I mentioned offline, sorry for not responding earlier. I missed the email for
the CL.

https://codereview.chromium.org/2862583010/diff/20001/chrome/browser/renderer...
File chrome/browser/renderer_context_menu/render_view_context_menu.cc (right):

https://codereview.chromium.org/2862583010/diff/20001/chrome/browser/renderer...
chrome/browser/renderer_context_menu/render_view_context_menu.cc:1175:
!embedder_web_contents_->IsSavable())) {
Can you add comments to explain this a bit? Something like:
// Full page pdf -> embedder is not savable, so we potentially show page items.
Otherwise we default to plugin items.
!embedder_web_contents_->IsSavable()

Also you can verify whether ^^^ is correct :)

https://codereview.chromium.org/2862583010/diff/20001/chrome/browser/renderer...
File
chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc
(right):

https://codereview.chromium.org/2862583010/diff/20001/chrome/browser/renderer...
chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc:721:
WebContents* inner_contents = web_contents_added_observer.GetWebContents();
Do you need to
ASSERT_TRUE(content::WaitForLoadStop(innner_contents));?

Yesterday, lukasza@ was seeing test flakiness because of this
(https://codereview.chromium.org/2871063002/).

https://codereview.chromium.org/2862583010/diff/20001/chrome/browser/renderer...
chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc:734:
ASSERT_FALSE(menu.IsItemPresent(IDC_BACK));
Here and above, back could be unavailable for other reasons like we don't have a
navigation item to go back to... IDC_RELOAD is better I think because it will
always be there.
However, since it's looking at its presence (IsItemPresent) instead of its
enabled-ness (IsCommandIdEnabled) this isn't wrong either. So consider this
optional.

https://codereview.chromium.org/2862583010/diff/20001/chrome/test/data/pdf/te...
File chrome/test/data/pdf/test-embedded-pdf.html (right):

https://codereview.chromium.org/2862583010/diff/20001/chrome/test/data/pdf/te...
chrome/test/data/pdf/test-embedded-pdf.html:9: var frame =
document.createElement("iframe");
I think embedded pdfs use <embed> instead of iframe. So I'd test both: embed tag
pdf here and iframe pdf in test-iframed-pdf.html

[Wei Li, 2017-05-11 21:38:48]

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

[commit-bot: I haz the power, 2017-05-11 21:39:58]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-11 23:34:11]

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

[commit-bot: I haz the power, 2017-05-11 23:34:12]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Wei Li, 2017-05-14 18:11:24]

Patchset #5 (id:80001) has been deleted

[Wei Li, 2017-05-16 22:52:24]

Description was changed from

==========
Save pdf file instead of web page through context menu for embedded pdf file

When user clicks 'Save as...' in the context menu over an embedded pdf file,
we should save the pdf file instead of the entire web page. We fix this by
showing
the correct context menu for embedded pdf versus full page pdf: only full page
pdf's context menu shows page navigation items such as 'Back' and 'Forward',
but embedded pdf's context menu doesn't show them.   

BUG=chromium:591071
==========

to

==========
Save pdf file instead of web page through context menu for embedded pdf file

When user clicks 'Save as...' in the context menu over an embedded pdf file,
we should save the pdf file instead of the entire web page. We fix this by
showing
the correct context menu for embedded pdf versus full page pdf: only full page
pdf's context menu shows page navigation items such as 'Reload',
but embedded pdf's context menu doesn't show them.   

BUG=chromium:591071
==========

[Wei Li, 2017-05-17 00:26:51]

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

[commit-bot: I haz the power, 2017-05-17 00:28:46]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-17 03:54:54]

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

[commit-bot: I haz the power, 2017-05-17 03:54:54]

Dry run: This issue passed the CQ dry run.

[Wei Li, 2017-05-17 05:37:10]

ptal, thanks

https://codereview.chromium.org/2862583010/diff/20001/chrome/browser/renderer...
File chrome/browser/renderer_context_menu/render_view_context_menu.cc (right):

https://codereview.chromium.org/2862583010/diff/20001/chrome/browser/renderer...
chrome/browser/renderer_context_menu/render_view_context_menu.cc:1175:
!embedder_web_contents_->IsSavable())) {
On 2017/05/11 18:15:17, lazyboy wrote:
> Can you add comments to explain this a bit? Something like:
> // Full page pdf -> embedder is not savable, so we potentially show page
items.
> Otherwise we default to plugin items.
> !embedder_web_contents_->IsSavable()
> 
> Also you can verify whether ^^^ is correct :)

Done.

https://codereview.chromium.org/2862583010/diff/20001/chrome/browser/renderer...
File
chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc
(right):

https://codereview.chromium.org/2862583010/diff/20001/chrome/browser/renderer...
chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc:721:
WebContents* inner_contents = web_contents_added_observer.GetWebContents();
On 2017/05/11 18:15:17, lazyboy wrote:
> Do you need to
> ASSERT_TRUE(content::WaitForLoadStop(innner_contents));?
> 
> Yesterday, lukasza@ was seeing test flakiness because of this
> (https://codereview.chromium.org/2871063002/). 

Turned out that I do need some kind of wait() to make sure the loading is done.
Changed.

https://codereview.chromium.org/2862583010/diff/20001/chrome/browser/renderer...
chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc:734:
ASSERT_FALSE(menu.IsItemPresent(IDC_BACK));
On 2017/05/11 18:15:17, lazyboy wrote:
> Here and above, back could be unavailable for other reasons like we don't have
a
> navigation item to go back to... IDC_RELOAD is better I think because it will
> always be there.
> However, since it's looking at its presence (IsItemPresent) instead of its
> enabled-ness (IsCommandIdEnabled) this isn't wrong either. So consider this
> optional.

I agree that enabled or not doesn't matter for this test case. But using
'reload' might help understanding. Done.

https://codereview.chromium.org/2862583010/diff/20001/chrome/test/data/pdf/te...
File chrome/test/data/pdf/test-embedded-pdf.html (right):

https://codereview.chromium.org/2862583010/diff/20001/chrome/test/data/pdf/te...
chrome/test/data/pdf/test-embedded-pdf.html:9: var frame =
document.createElement("iframe");
On 2017/05/11 18:15:17, lazyboy wrote:
> I think embedded pdfs use <embed> instead of iframe. So I'd test both: embed
tag
> pdf here and iframe pdf in test-iframed-pdf.html

The word 'embedded' came in naturally for non-full page pdf. Finding a
replacement doesn't seem easy. Pls see whether my change is good enough.

[lazyboy, 2017-05-17 16:34:45]

https://codereview.chromium.org/2862583010/diff/20001/chrome/browser/renderer...
File
chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc
(right):

https://codereview.chromium.org/2862583010/diff/20001/chrome/browser/renderer...
chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc:721:
WebContents* inner_contents = web_contents_added_observer.GetWebContents();
On 2017/05/17 05:37:09, Wei Li wrote:
> On 2017/05/11 18:15:17, lazyboy wrote:
> > Do you need to
> > ASSERT_TRUE(content::WaitForLoadStop(innner_contents));?
> > 
> > Yesterday, lukasza@ was seeing test flakiness because of this
> > (https://codereview.chromium.org/2871063002/). 
> 
> Turned out that I do need some kind of wait() to make sure the loading is
done.
> Changed.

I can't find where you've added the wait in the new patch set. Can you explain?
Also, add a comment where we are waiting:
// Wait for .. to finish loading.
etc.

https://codereview.chromium.org/2862583010/diff/120001/chrome/browser/rendere...
File
chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc
(right):

https://codereview.chromium.org/2862583010/diff/120001/chrome/browser/rendere...
chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc:170:
"setTimeout(\"var l = document.getElementById('link1');"
Is this setTimeout necessary? Add a comment explaining the reason if it is so.
Generally avoid these if possible because they hint that we could still hit race
condition at times.

https://codereview.chromium.org/2862583010/diff/120001/chrome/test/data/pdf/t...
File chrome/test/data/pdf/test-embed-pdf.html (right):

https://codereview.chromium.org/2862583010/diff/120001/chrome/test/data/pdf/t...
chrome/test/data/pdf/test-embed-pdf.html:5: <div id="div1"></div>
nit:s/div1/container (in both html)

https://codereview.chromium.org/2862583010/diff/120001/chrome/test/data/pdf/t...
chrome/test/data/pdf/test-embed-pdf.html:9: var frame =
document.createElement("embed");
Since this is not a frame, call it embedElement or something.

https://codereview.chromium.org/2862583010/diff/120001/chrome/test/data/pdf/t...
chrome/test/data/pdf/test-embed-pdf.html:11: var outter_element =
document.getElementById("div1");
nit: outerElement (also applies to other file).

[Wei Li, 2017-05-18 00:48:11]

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

[commit-bot: I haz the power, 2017-05-18 00:50:20]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-18 05:06:51]

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

[commit-bot: I haz the power, 2017-05-18 05:06:52]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Wei Li, 2017-05-18 20:30:14]

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

[commit-bot: I haz the power, 2017-05-18 20:31:37]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-18 22:50:42]

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

[commit-bot: I haz the power, 2017-05-18 22:50:43]

Dry run: This issue passed the CQ dry run.

[Wei Li, 2017-05-18 23:25:28]

ptal, thanks

https://codereview.chromium.org/2862583010/diff/120001/chrome/browser/rendere...
File
chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc
(right):

https://codereview.chromium.org/2862583010/diff/120001/chrome/browser/rendere...
chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc:170:
"setTimeout(\"var l = document.getElementById('link1');"
On 2017/05/17 16:34:44, lazyboy wrote:
> Is this setTimeout necessary? Add a comment explaining the reason if it is so.
> Generally avoid these if possible because they hint that we could still hit
race
> condition at times.

Since wait() functions are added, I think this can be removed. Done.

https://codereview.chromium.org/2862583010/diff/120001/chrome/test/data/pdf/t...
File chrome/test/data/pdf/test-embed-pdf.html (right):

https://codereview.chromium.org/2862583010/diff/120001/chrome/test/data/pdf/t...
chrome/test/data/pdf/test-embed-pdf.html:5: <div id="div1"></div>
On 2017/05/17 16:34:44, lazyboy wrote:
> nit:s/div1/container (in both html)

Done.

https://codereview.chromium.org/2862583010/diff/120001/chrome/test/data/pdf/t...
chrome/test/data/pdf/test-embed-pdf.html:9: var frame =
document.createElement("embed");
On 2017/05/17 16:34:44, lazyboy wrote:
> Since this is not a frame, call it embedElement or something.

Done.

https://codereview.chromium.org/2862583010/diff/120001/chrome/test/data/pdf/t...
chrome/test/data/pdf/test-embed-pdf.html:11: var outter_element =
document.getElementById("div1");
On 2017/05/17 16:34:44, lazyboy wrote:
> nit: outerElement (also applies to other file).

Done.

[lazyboy, 2017-05-18 23:38:56]

lgtm, thanks.

[Wei Li, 2017-05-19 03:44:57]

The CQ bit was checked by weili@chromium.org

[commit-bot: I haz the power, 2017-05-19 03:45:42]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-19 06:04:12]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1495165497504630,
"parent_rev": "6211b4b605a164394225e2fb02c855708924a9d2", "commit_rev":
"fd0e534843ac5926c1b26c7e1e7a7229fcb3aeee"}

[commit-bot: I haz the power, 2017-05-19 06:04:26]

Description was changed from

==========
Save pdf file instead of web page through context menu for embedded pdf file

When user clicks 'Save as...' in the context menu over an embedded pdf file,
we should save the pdf file instead of the entire web page. We fix this by
showing
the correct context menu for embedded pdf versus full page pdf: only full page
pdf's context menu shows page navigation items such as 'Reload',
but embedded pdf's context menu doesn't show them.   

BUG=chromium:591071
==========

to

==========
Save pdf file instead of web page through context menu for embedded pdf file

When user clicks 'Save as...' in the context menu over an embedded pdf file,
we should save the pdf file instead of the entire web page. We fix this by
showing
the correct context menu for embedded pdf versus full page pdf: only full page
pdf's context menu shows page navigation items such as 'Reload',
but embedded pdf's context menu doesn't show them.   

BUG=chromium:591071

Review-Url: https://codereview.chromium.org/2862583010
Cr-Commit-Position: refs/heads/master@{#473105}
Committed:
https://chromium.googlesource.com/chromium/src/+/fd0e534843ac5926c1b26c7e1e7a...
==========

[commit-bot: I haz the power, 2017-05-19 06:04:27]

Committed patchset #8 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/fd0e534843ac5926c1b26c7e1e7a...

[Marc Treib, 2017-05-19 12:58:08]

A revert of this CL (patchset #8 id:160001) has been created in
https://codereview.chromium.org/2891923005/ by treib@chromium.org.

The reason for reverting is:
PdfPluginContextMenuBrowserTest.EmbeddedPdfHasNoPageItems is flaky on Windows,
see:
https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType....

[Wei Li, 2017-05-19 19:18:33]

Description was changed from

==========
Save pdf file instead of web page through context menu for embedded pdf file

When user clicks 'Save as...' in the context menu over an embedded pdf file,
we should save the pdf file instead of the entire web page. We fix this by
showing
the correct context menu for embedded pdf versus full page pdf: only full page
pdf's context menu shows page navigation items such as 'Reload',
but embedded pdf's context menu doesn't show them.   

BUG=chromium:591071

Review-Url: https://codereview.chromium.org/2862583010
Cr-Commit-Position: refs/heads/master@{#473105}
Committed:
https://chromium.googlesource.com/chromium/src/+/fd0e534843ac5926c1b26c7e1e7a...
==========

to

==========
Add type for embedded element, and also use setTimeout in javascript.
Hopefully this will avoid test flakiness due to timeout.

>Save pdf file instead of web page through context menu for embedded pdf file

>When user clicks 'Save as...' in the context menu over an embedded pdf file,
>we should save the pdf file instead of the entire web page. We fix this by
showing
>the correct context menu for embedded pdf versus full page pdf: only full page
>pdf's context menu shows page navigation items such as 'Reload',
>but embedded pdf's context menu doesn't show them.   

>BUG=chromium:591071

>Review-Url: https://codereview.chromium.org/2862583010
>Cr-Commit-Position: refs/heads/master@{#473105}
>Committed:
>https://chromium.googlesource.com/chromium/src/+/fd0e534843ac5926c1b26c7e1e7a7229fcb3aeee
==========

[Wei Li, 2017-05-19 19:19:08]

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

[commit-bot: I haz the power, 2017-05-19 19:20:23]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-19 21:45:12]

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

[commit-bot: I haz the power, 2017-05-19 21:45:13]

Dry run: This issue passed the CQ dry run.

[Wei Li, 2017-05-19 21:48:01]

Description was changed from

==========
Add type for embedded element, and also use setTimeout in javascript.
Hopefully this will avoid test flakiness due to timeout.

>Save pdf file instead of web page through context menu for embedded pdf file

>When user clicks 'Save as...' in the context menu over an embedded pdf file,
>we should save the pdf file instead of the entire web page. We fix this by
showing
>the correct context menu for embedded pdf versus full page pdf: only full page
>pdf's context menu shows page navigation items such as 'Reload',
>but embedded pdf's context menu doesn't show them.   

>BUG=chromium:591071

>Review-Url: https://codereview.chromium.org/2862583010
>Cr-Commit-Position: refs/heads/master@{#473105}
>Committed:
>https://chromium.googlesource.com/chromium/src/+/fd0e534843ac5926c1b26c7e1e7a7229fcb3aeee
==========

to

==========
Use setTimeout() in javascript to make sure DOM is fully loaded. 
Also add type for embedded element to make it clearer.
Hopefully this will avoid test flakiness due to timeout.

>Save pdf file instead of web page through context menu for embedded pdf file

>When user clicks 'Save as...' in the context menu over an embedded pdf file,
>we should save the pdf file instead of the entire web page. We fix this by
showing
>the correct context menu for embedded pdf versus full page pdf: only full page
>pdf's context menu shows page navigation items such as 'Reload',
>but embedded pdf's context menu doesn't show them.   

>BUG=chromium:591071

>Review-Url: https://codereview.chromium.org/2862583010
>Cr-Commit-Position: refs/heads/master@{#473105}
>Committed:
>https://chromium.googlesource.com/chromium/src/+/fd0e534843ac5926c1b26c7e1e7a7229fcb3aeee
==========

[Wei Li, 2017-05-19 21:50:48]

Patch 10 is the new change. ptal, thanks!

[Wei Li, 2017-05-22 06:42:17]

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

[commit-bot: I haz the power, 2017-05-22 06:42:32]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Wei Li, 2017-05-22 06:44:24]

Patchset #12 (id:240001) has been deleted

[Wei Li, 2017-05-22 15:59:22]

Description was changed from

==========
Use setTimeout() in javascript to make sure DOM is fully loaded. 
Also add type for embedded element to make it clearer.
Hopefully this will avoid test flakiness due to timeout.

>Save pdf file instead of web page through context menu for embedded pdf file

>When user clicks 'Save as...' in the context menu over an embedded pdf file,
>we should save the pdf file instead of the entire web page. We fix this by
showing
>the correct context menu for embedded pdf versus full page pdf: only full page
>pdf's context menu shows page navigation items such as 'Reload',
>but embedded pdf's context menu doesn't show them.   

>BUG=chromium:591071

>Review-Url: https://codereview.chromium.org/2862583010
>Cr-Commit-Position: refs/heads/master@{#473105}
>Committed:
>https://chromium.googlesource.com/chromium/src/+/fd0e534843ac5926c1b26c7e1e7a7229fcb3aeee
==========

to

==========
Remove the flaky test as the flaky test is not really necessary. 

>Save pdf file instead of web page through context menu for embedded pdf file

>When user clicks 'Save as...' in the context menu over an embedded pdf file,
>we should save the pdf file instead of the entire web page. We fix this by
showing
>the correct context menu for embedded pdf versus full page pdf: only full page
>pdf's context menu shows page navigation items such as 'Reload',
>but embedded pdf's context menu doesn't show them.   

>BUG=chromium:591071

>Review-Url: https://codereview.chromium.org/2862583010
>Cr-Commit-Position: refs/heads/master@{#473105}
>Committed:
>https://chromium.googlesource.com/chromium/src/+/fd0e534843ac5926c1b26c7e1e7a7229fcb3aeee
==========

[Wei Li, 2017-05-22 16:01:42]

On a second thought, I think it is easier and safer to just remove the flaky
embeddedPdf test. This test is not really necessary since we still have another
test testing pdf inside page case.

ptal, thanks

[lazyboy, 2017-05-22 17:25:10]

On 2017/05/22 16:01:42, Wei Li wrote:
> On a second thought, I think it is easier and safer to just remove the flaky
> embeddedPdf test. This test is not really necessary since we still have
another
> test testing pdf inside page case.

OK, no setTimeouts, lgtm.
> 
> ptal, thanks

[Wei Li, 2017-05-22 18:06:50]

The CQ bit was checked by weili@chromium.org

[commit-bot: I haz the power, 2017-05-22 18:07:44]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-22 18:12:36]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1495476410032680,
"parent_rev": "d75c92bc39687a520bf3e67ae29fb54f9a6bb01e", "commit_rev":
"85a3174dfc2310377872a88db75f7370fa2f9734"}

[commit-bot: I haz the power, 2017-05-22 18:12:48]

Description was changed from

==========
Remove the flaky test as the flaky test is not really necessary. 

>Save pdf file instead of web page through context menu for embedded pdf file

>When user clicks 'Save as...' in the context menu over an embedded pdf file,
>we should save the pdf file instead of the entire web page. We fix this by
showing
>the correct context menu for embedded pdf versus full page pdf: only full page
>pdf's context menu shows page navigation items such as 'Reload',
>but embedded pdf's context menu doesn't show them.   

>BUG=chromium:591071

>Review-Url: https://codereview.chromium.org/2862583010
>Cr-Commit-Position: refs/heads/master@{#473105}
>Committed:
>https://chromium.googlesource.com/chromium/src/+/fd0e534843ac5926c1b26c7e1e7a7229fcb3aeee
==========

to

==========
Remove the flaky test as the flaky test is not really necessary. 

>Save pdf file instead of web page through context menu for embedded pdf file

>When user clicks 'Save as...' in the context menu over an embedded pdf file,
>we should save the pdf file instead of the entire web page. We fix this by
showing
>the correct context menu for embedded pdf versus full page pdf: only full page
>pdf's context menu shows page navigation items such as 'Reload',
>but embedded pdf's context menu doesn't show them.   

>BUG=chromium:591071

>Review-Url: https://codereview.chromium.org/2862583010
>Cr-Commit-Position: refs/heads/master@{#473105}
>Committed:
>https://chromium.googlesource.com/chromium/src/+/fd0e534843ac5926c1b26c7e1e7a7229fcb3aeee

Review-Url: https://codereview.chromium.org/2862583010
Cr-Commit-Position: refs/heads/master@{#473625}
Committed:
https://chromium.googlesource.com/chromium/src/+/85a3174dfc2310377872a88db75f...
==========

[commit-bot: I haz the power, 2017-05-22 18:12:49]

Committed patchset #11 (id:220001) as
https://chromium.googlesource.com/chromium/src/+/85a3174dfc2310377872a88db75f...