PlzNavigate: make MHTML iframe load working.

PlzNavigate: make MHTML iframe load working.

This CL makes the renderer use the MHTML Archive when there is one for loading
iframes. Some tests are added.
Design doc: https://goo.gl/jCVFBc

BUG=705508
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_browser_side_navigation_rel

Review-Url: https://codereview.chromium.org/2834013002
Cr-Commit-Position: refs/heads/master@{#470081}
Committed: https://chromium.googlesource.com/chromium/src/+/8d745a7f38b559d75efe5b04a79f5fecd89a2e22

[arthursonzogni, 2017-04-21 09:33:18]

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

[commit-bot: I haz the power, 2017-04-21 09:33:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[arthursonzogni, 2017-04-21 09:38:34]

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

[commit-bot: I haz the power, 2017-04-21 09:38:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[arthursonzogni, 2017-04-21 09:39:31]

Description was changed from

==========
PlzNavigate: make MHTML working.

This CL makes the renderer use the MHTML Archive when there is one. Some tests
are added.
Design doc: https://goo.gl/jCVFBc

BUG=705508
==========

to

==========
PlzNavigate: make MHTML working.

This CL makes the renderer use the MHTML Archive when there is one. Some tests
are added.
Design doc: https://goo.gl/jCVFBc

BUG=705508

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_browser_side_navigation_rel
==========

[commit-bot: I haz the power, 2017-04-21 10:37:52]

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

[commit-bot: I haz the power, 2017-04-21 10:37:53]

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

[arthursonzogni, 2017-04-25 15:09:17]

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

[commit-bot: I haz the power, 2017-04-25 15:09:45]

Dry run: CQ is trying da patch.

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

[arthursonzogni, 2017-04-25 16:30:29]

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

[commit-bot: I haz the power, 2017-04-25 16:30:44]

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-04-25 16:42:50]

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

[commit-bot: I haz the power, 2017-04-25 16:42:51]

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

[arthursonzogni, 2017-05-02 12:29:47]

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

[commit-bot: I haz the power, 2017-05-02 12:30:03]

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-02 12:41:26]

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

[commit-bot: I haz the power, 2017-05-02 12:41:27]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[arthursonzogni, 2017-05-02 12:51:34]

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

[commit-bot: I haz the power, 2017-05-02 12:51: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-02 13:41:27]

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

[commit-bot: I haz the power, 2017-05-02 13:41:28]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[arthursonzogni, 2017-05-03 08:57:16]

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

[commit-bot: I haz the power, 2017-05-03 08:57:31]

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-03 09:05:39]

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

[commit-bot: I haz the power, 2017-05-03 09:05:40]

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

[arthursonzogni, 2017-05-03 09:25:41]

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

[commit-bot: I haz the power, 2017-05-03 09:25:56]

Dry run: CQ is trying da patch.

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

[arthursonzogni, 2017-05-03 09:26:09]

Description was changed from

==========
PlzNavigate: make MHTML working.

This CL makes the renderer use the MHTML Archive when there is one. Some tests
are added.
Design doc: https://goo.gl/jCVFBc

BUG=705508

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_browser_side_navigation_rel
==========

to

==========
PlzNavigate: make MHTML working.

This CL makes the renderer use the MHTML Archive when there is one for loading
iframes. Some tests
are added.
Design doc: https://goo.gl/jCVFBc

BUG=705508

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_browser_side_navigation_rel
==========

[commit-bot: I haz the power, 2017-05-03 11:00:22]

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

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

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

[arthursonzogni, 2017-05-03 11:08:40]

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

[commit-bot: I haz the power, 2017-05-03 11:08:53]

Dry run: CQ is trying da patch.

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

[arthursonzogni, 2017-05-03 13:12:58]

Patchset #1 (id:1) has been deleted

[arthursonzogni, 2017-05-03 13:13:08]

Patchset #1 (id:20001) has been deleted

[arthursonzogni, 2017-05-03 13:13:18]

Patchset #1 (id:40001) has been deleted

[arthursonzogni, 2017-05-03 13:13:27]

Patchset #1 (id:60001) has been deleted

[arthursonzogni, 2017-05-03 13:13:33]

Patchset #1 (id:80001) has been deleted

[arthursonzogni, 2017-05-03 13:13:41]

Patchset #1 (id:100001) has been deleted

[arthursonzogni, 2017-05-03 13:13:53]

Patchset #1 (id:120001) has been deleted

[commit-bot: I haz the power, 2017-05-03 13:20:57]

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

[commit-bot: I haz the power, 2017-05-03 13:20:58]

Dry run: This issue passed the CQ dry run.

[arthursonzogni, 2017-05-03 13:48:44]

Description was changed from

==========
PlzNavigate: make MHTML working.

This CL makes the renderer use the MHTML Archive when there is one for loading
iframes. Some tests
are added.
Design doc: https://goo.gl/jCVFBc

BUG=705508

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_browser_side_navigation_rel
==========

to

==========
PlzNavigate: make MHTML iframe load working.

This CL makes the renderer use the MHTML Archive when there is one for loading
iframes. Some tests are added.
Design doc: https://goo.gl/jCVFBc

BUG=705508

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_browser_side_navigation_rel
==========

[arthursonzogni, 2017-05-03 14:03:30]

arthursonzogni@chromium.org changed reviewers:
+ japhet@chromium.org, jianli@chromium.org, nasko@chromium.org

[arthursonzogni, 2017-05-03 14:03:31]

Hi Jian, Nate and Nasko,
Please take a look.

You are owner of:
jianli: third_party/WebKit/LayoutTests/mhtml/
japhet: third_party/WebKit/Source/{core, web}/
nasko:  content/

Thanks in advance!
Arthur

[carlosk, 2017-05-03 16:56:48]

carlosk@chromium.org changed reviewers:
+ carlosk@chromium.org

[carlosk, 2017-05-03 16:56:49]

Thanks for this fix!

https://codereview.chromium.org/2834013002/diff/140001/content/common/navigat...
File content/common/navigation_params_unittest.cc (right):

https://codereview.chromium.org/2834013002/diff/140001/content/common/navigat...
content/common/navigation_params_unittest.cc:20:
EXPECT_TRUE(ShouldMakeNetworkRequestForURL(GURL("cid:foo@bar")));
nit: Please add a comment explaining why cid: and data: URLs should return
|true| here even though they don't actually generate actual network requests.
It could be something in the lines of: "Even though data: and cid: schemes do
not generate actual network requests they must be handled by the network stack
and so must return true for this check."

https://codereview.chromium.org/2834013002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/LocalFrameClientImpl.cpp (right):

https://codereview.chromium.org/2834013002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/LocalFrameClientImpl.cpp:78: #include
"platform/mhtml/MHTMLArchive.h"
Do you really need this include?

[nasko, 2017-05-03 17:28:39]

https://codereview.chromium.org/2834013002/diff/140001/content/common/navigat...
File content/common/navigation_params.cc (left):

https://codereview.chromium.org/2834013002/diff/140001/content/common/navigat...
content/common/navigation_params.cc:26: url.SchemeIs(url::kContentIDScheme) ||
url == content::kAboutSrcDocURL) {
The comments in this CL say that "cid:" cannot be encountered outside of MTHML.
Those don't go to the network stack, so why are we changing this method to
return true for it?

https://codereview.chromium.org/2834013002/diff/140001/content/renderer/rende...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/2834013002/diff/140001/content/renderer/rende...
content/renderer/render_frame_impl.cc:5611: ShouldMakeNetworkRequestForURL(url)
&& !use_archive) {
I'm not sure I follow why we should be sending "cid:Something" requests to the
network stack. Since we don't expect them outside of MHTML, shouldn't we have a
simple check that says "if !archive && cid, return ignore" (or if there is a
better policy value)?

[jianli, 2017-05-03 22:56:19]

https://codereview.chromium.org/2834013002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/LocalFrameClientImpl.h (right):

https://codereview.chromium.org/2834013002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/LocalFrameClientImpl.h:234: bool
ShouldUseMHTMLArchiveForSubFrame() const;
When an archive exist, we should always use it to load a subframe. So probably
it is better to name this method as something like IsLoadedAsMHTMLArchive?

Note that exposing this is very useful in probably helping other business,
consider we might want to add some mhtml specific logic in renderer client. So I
would like to see this name be more generic.

[arthursonzogni, 2017-05-04 12:27:36]

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

[commit-bot: I haz the power, 2017-05-04 12:27:45]

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-04 14:15:31]

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

[commit-bot: I haz the power, 2017-05-04 14:15:32]

Dry run: This issue passed the CQ dry run.

[arthursonzogni, 2017-05-04 14:26:32]

Thanks for your comments. Here is a new patch.

https://codereview.chromium.org/2834013002/diff/140001/content/common/navigat...
File content/common/navigation_params.cc (left):

https://codereview.chromium.org/2834013002/diff/140001/content/common/navigat...
content/common/navigation_params.cc:26: url.SchemeIs(url::kContentIDScheme) ||
url == content::kAboutSrcDocURL) {
On 2017/05/03 17:28:39, nasko wrote:
> The comments in this CL say that "cid:" cannot be encountered outside of
MTHML.
> Those don't go to the network stack, so why are we changing this method to
> return true for it?

You are right, the "cid:" cannot be encountered outside of a MHTML Archive.
Without PlzNavigate, when there is no MHTML Archive, the request will continue
inside the ResourceFetcher and the renderer will ask the browser for the
resource. Doing this modification in ShouldMakeNetworkRequestForURL leads to
have the same behavior with PlzNavigate. E.G. using the browser to open the
"cid:" url.
The user will be asked which application he/she want to use to open the "cid:"
URL.

Alternatively, we can make it always use the ResourceFetcher and I can add a
line of code that make the request to be ignored if there is no archives and the
url protocol is "cid:"
I will do it in the next patch. Please tell what do you prefer.

https://codereview.chromium.org/2834013002/diff/140001/content/common/navigat...
File content/common/navigation_params_unittest.cc (right):

https://codereview.chromium.org/2834013002/diff/140001/content/common/navigat...
content/common/navigation_params_unittest.cc:20:
EXPECT_TRUE(ShouldMakeNetworkRequestForURL(GURL("cid:foo@bar")));
On 2017/05/03 16:56:49, carlosk wrote:
> nit: Please add a comment explaining why cid: and data: URLs should return
> |true| here even though they don't actually generate actual network requests.
> It could be something in the lines of: "Even though data: and cid: schemes do
> not generate actual network requests they must be handled by the network stack
> and so must return true for this check."

Yes, really good idea, especially for "data:".
Done.

https://codereview.chromium.org/2834013002/diff/140001/content/renderer/rende...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/2834013002/diff/140001/content/renderer/rende...
content/renderer/render_frame_impl.cc:5611: ShouldMakeNetworkRequestForURL(url)
&& !use_archive) {
On 2017/05/03 17:28:39, nasko wrote:
> I'm not sure I follow why we should be sending "cid:Something" requests to the
> network stack. Since we don't expect them outside of MHTML, shouldn't we have
a
> simple check that says "if !archive && cid, return ignore" (or if there is a
> better policy value)?

See my first answer above.

https://codereview.chromium.org/2834013002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/LocalFrameClientImpl.cpp (right):

https://codereview.chromium.org/2834013002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/LocalFrameClientImpl.cpp:78: #include
"platform/mhtml/MHTMLArchive.h"
On 2017/05/03 16:56:49, carlosk wrote:
> Do you really need this include?

Yes, except for the last one. Thanks!

https://codereview.chromium.org/2834013002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/LocalFrameClientImpl.h (right):

https://codereview.chromium.org/2834013002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/LocalFrameClientImpl.h:234: bool
ShouldUseMHTMLArchiveForSubFrame() const;
On 2017/05/03 22:56:19, jianli wrote:
> When an archive exist, we should always use it to load a subframe. So probably
> it is better to name this method as something like IsLoadedAsMHTMLArchive?
> 
> Note that exposing this is very useful in probably helping other business,
> consider we might want to add some mhtml specific logic in renderer client. So
I
> would like to see this name be more generic.

Done.

[nasko, 2017-05-04 15:48:18]

https://codereview.chromium.org/2834013002/diff/140001/content/common/navigat...
File content/common/navigation_params.cc (left):

https://codereview.chromium.org/2834013002/diff/140001/content/common/navigat...
content/common/navigation_params.cc:26: url.SchemeIs(url::kContentIDScheme) ||
url == content::kAboutSrcDocURL) {
On 2017/05/04 14:26:32, arthursonzogni wrote:
> On 2017/05/03 17:28:39, nasko wrote:
> > The comments in this CL say that "cid:" cannot be encountered outside of
> MTHML.
> > Those don't go to the network stack, so why are we changing this method to
> > return true for it?
> 
> You are right, the "cid:" cannot be encountered outside of a MHTML Archive.
> Without PlzNavigate, when there is no MHTML Archive, the request will continue
> inside the ResourceFetcher and the renderer will ask the browser for the
> resource. Doing this modification in ShouldMakeNetworkRequestForURL leads to
> have the same behavior with PlzNavigate. E.G. using the browser to open the
> "cid:" url.

Would the cid: scheme have a valid URL as the rest of the string? If not, then
there is no reason to send it up to the browser, as it will not be able to open
it correctly.

> The user will be asked which application he/she want to use to open the "cid:"
> URL.

Isn't this logically handled by LoadURLExternally?

> Alternatively, we can make it always use the ResourceFetcher and I can add a
> line of code that make the request to be ignored if there is no archives and
the
> url protocol is "cid:"
> I will do it in the next patch. Please tell what do you prefer.

The answer will depend on whether cid:<...> can have a valid URL as <...>, which
I don't know of the top of my head.

https://codereview.chromium.org/2834013002/diff/160001/content/common/navigat...
File content/common/navigation_params_unittest.cc (right):

https://codereview.chromium.org/2834013002/diff/160001/content/common/navigat...
content/common/navigation_params_unittest.cc:19: // Even though a "data" url
doesn't generate actual network requests. It is
nit: "data:", since it is a scheme. "requests, it is" (one sentence).
Let's also include some context if we are adding a comment giving an example why
- some data: URLs result in downloads, which are handled in the network stack.

[arthursonzogni, 2017-05-05 09:01:06]

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

[commit-bot: I haz the power, 2017-05-05 09:01:18]

Dry run: CQ is trying da patch.

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

[arthursonzogni, 2017-05-05 09:07:47]

https://codereview.chromium.org/2834013002/diff/140001/content/common/navigat...
File content/common/navigation_params.cc (left):

https://codereview.chromium.org/2834013002/diff/140001/content/common/navigat...
content/common/navigation_params.cc:26: url.SchemeIs(url::kContentIDScheme) ||
url == content::kAboutSrcDocURL) {
On 2017/05/04 15:48:18, nasko wrote:
> On 2017/05/04 14:26:32, arthursonzogni wrote:
> > On 2017/05/03 17:28:39, nasko wrote:
> > > The comments in this CL say that "cid:" cannot be encountered outside of
> > MTHML.
> > > Those don't go to the network stack, so why are we changing this method to
> > > return true for it?
> > 
> > You are right, the "cid:" cannot be encountered outside of a MHTML Archive.
> > Without PlzNavigate, when there is no MHTML Archive, the request will
continue
> > inside the ResourceFetcher and the renderer will ask the browser for the
> > resource. Doing this modification in ShouldMakeNetworkRequestForURL leads to
> > have the same behavior with PlzNavigate. E.G. using the browser to open the
> > "cid:" url.
> 
> Would the cid: scheme have a valid URL as the rest of the string? If not, then
> there is no reason to send it up to the browser, as it will not be able to
open
> it correctly.
It will not have a valid URL as the rest of the string. After cid: there is a
uniquely identified string that should match the content of the "Content-ID"
header of a resource in the MHTML Archive.
> > The user will be asked which application he/she want to use to open the
"cid:"
> > URL.
> 
> Isn't this logically handled by LoadURLExternally?
I don't think so. I have checked.
> 
> > Alternatively, we can make it always use the ResourceFetcher and I can add a
> > line of code that make the request to be ignored if there is no archives and
> the
> > url protocol is "cid:"
> > I will do it in the next patch. Please tell what do you prefer.
> 
> The answer will depend on whether cid:<...> can have a valid URL as <...>,
which
> I don't know of the top of my head.
AFAIK, I don't think.

https://codereview.chromium.org/2834013002/diff/160001/content/common/navigat...
File content/common/navigation_params_unittest.cc (right):

https://codereview.chromium.org/2834013002/diff/160001/content/common/navigat...
content/common/navigation_params_unittest.cc:19: // Even though a "data" url
doesn't generate actual network requests. It is
On 2017/05/04 15:48:18, nasko wrote:
> nit: "data:", since it is a scheme. "requests, it is" (one sentence).
> Let's also include some context if we are adding a comment giving an example
why
> - some data: URLs result in downloads, which are handled in the network stack.

I will list some reasons why some data-urls require using the network stack.

[commit-bot: I haz the power, 2017-05-05 10:53:20]

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

[commit-bot: I haz the power, 2017-05-05 10:53:22]

Dry run: This issue passed the CQ dry run.

[arthursonzogni, 2017-05-05 14:07:26]

Patchset #4 (id:200001) has been deleted

[Nate Chapin, 2017-05-05 20:34:18]

lgtm

https://codereview.chromium.org/2834013002/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/LocalFrameClientImpl.cpp (right):

https://codereview.chromium.org/2834013002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/LocalFrameClientImpl.cpp:519: if
(IsLoadedAsMHTMLArchive()) {
Could we inline this and share some logic with
is_history_navigation_in_new_child_frame above?

At least getting the parent LocalFrame* (or setting it to nullptr if there's no
parent or it's not local) would make both pieces more readable I think.

[nasko, 2017-05-05 20:43:11]

content/ LGTM

[pfeldman, 2017-05-05 20:45:35]

pfeldman@chromium.org changed reviewers:
+ pfeldman@chromium.org

[pfeldman, 2017-05-05 20:45:36]

WebKit/ lgtm

https://codereview.chromium.org/2834013002/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
(right):

https://codereview.chromium.org/2834013002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp:583: if
(resource_request.Url().ProtocolIs(kContentIdScheme) && !archive_) {
[perf]: check !archive_ first
[style nit]: drop {}

[nasko, 2017-05-05 20:46:55]

The CQ bit was checked by nasko@chromium.org

[commit-bot: I haz the power, 2017-05-05 20:48:14]

CQ is trying da patch.

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

[jianli, 2017-05-05 20:51:21]

lgtm

[commit-bot: I haz the power, 2017-05-05 20:54:13]

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

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

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jam, 2017-05-08 17:13:36]

The CQ bit was checked by jam@chromium.org

[jam, 2017-05-08 17:13:37]

The patchset sent to the CQ was uploaded after l-g-t-m from japhet@chromium.org,
jianli@chromium.org, pfeldman@chromium.org, nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/2834013002/#ps240001
(title: "rebase")

[commit-bot: I haz the power, 2017-05-08 17:13:57]

CQ is trying da patch.

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

[jam, 2017-05-08 17:15:33]

The CQ bit was unchecked by jam@chromium.org

[nasko, 2017-05-08 17:18:09]

The CQ bit was checked by nasko@chromium.org

[commit-bot: I haz the power, 2017-05-08 17:18:34]

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-08 19:48:07]

CQ is committing da patch.

Bot data: {"patchset_id": 240001, "attempt_start_ts": 1494263889164330,
"parent_rev": "5f3484435c495bdc313e8cf1628a200b927f1209", "commit_rev":
"8d745a7f38b559d75efe5b04a79f5fecd89a2e22"}

[commit-bot: I haz the power, 2017-05-08 19:48:36]

Description was changed from

==========
PlzNavigate: make MHTML iframe load working.

This CL makes the renderer use the MHTML Archive when there is one for loading
iframes. Some tests are added.
Design doc: https://goo.gl/jCVFBc

BUG=705508

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_browser_side_navigation_rel
==========

to

==========
PlzNavigate: make MHTML iframe load working.

This CL makes the renderer use the MHTML Archive when there is one for loading
iframes. Some tests are added.
Design doc: https://goo.gl/jCVFBc

BUG=705508

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_browser_side_navigation_rel

Review-Url: https://codereview.chromium.org/2834013002
Cr-Commit-Position: refs/heads/master@{#470081}
Committed:
https://chromium.googlesource.com/chromium/src/+/8d745a7f38b559d75efe5b04a79f...
==========

[commit-bot: I haz the power, 2017-05-08 19:48:38]

Committed patchset #5 (id:240001) as
https://chromium.googlesource.com/chromium/src/+/8d745a7f38b559d75efe5b04a79f...