Support serializing shadow DOM to MHTML

Support serializing shadow DOM to MHTML

The content of shadow DOM tree will be serialized and put within a template element that appends to the children of the shadow host element. The template element contains a special attribute to denote the mode of the shadow dom tree.

When MHTML page is being loaded, the shadow DOM tree will be recreated by the injected script to convert the templates to the shadow DOM trees.

Please see the bug for the link to the design doc.

BUG=695738
TEST=new tests added

Review-Url: https://codereview.chromium.org/2912213002
Cr-Commit-Position: refs/heads/master@{#479550}
Committed: https://chromium.googlesource.com/chromium/src/+/05d415724da48cbeba542558a3ce5689ec0fee93

[jianli, 2017-05-30 23:54:33]

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

[commit-bot: I haz the power, 2017-05-30 23:55:18]

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-31 01:57:02]

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

[commit-bot: I haz the power, 2017-05-31 01:57:04]

Dry run: This issue passed the CQ dry run.

[jianli, 2017-05-31 21:38:07]

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

[commit-bot: I haz the power, 2017-05-31 21:39:28]

Dry run: CQ is trying da patch.

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

[jianli, 2017-05-31 21:43:26]

Description was changed from

==========
Support serializing shadow DOM to MHTML

BUG=695738
==========

to

==========
Support serializing shadow DOM to MHTML

The content of shadow DOM tree will be serialized and put within a template
element that appends to the children of the shadow host element. The template
element contains a special attribute to denote the mode of the shadow dom tree.

When MHTML page is being loaded, the shadow DOM tree will be recreated by the
injected script to convert the templates to the shadow DOM trees.

BUG=695738
==========

[jianli, 2017-05-31 23:19:16]

Description was changed from

==========
Support serializing shadow DOM to MHTML

The content of shadow DOM tree will be serialized and put within a template
element that appends to the children of the shadow host element. The template
element contains a special attribute to denote the mode of the shadow dom tree.

When MHTML page is being loaded, the shadow DOM tree will be recreated by the
injected script to convert the templates to the shadow DOM trees.

BUG=695738
==========

to

==========
Support serializing shadow DOM to MHTML

The content of shadow DOM tree will be serialized and put within a template
element that appends to the children of the shadow host element. The template
element contains a special attribute to denote the mode of the shadow dom tree.

When MHTML page is being loaded, the shadow DOM tree will be recreated by the
injected script to convert the templates to the shadow DOM trees.

Please see the bug for the link to the design doc.

BUG=695738
TEST=new tests added
==========

[jianli, 2017-05-31 23:20:26]

jianli@chromium.org changed reviewers:
+ dimich@chromium.org, mkwst@chromium.org

[jianli, 2017-05-31 23:20:39]

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

[commit-bot: I haz the power, 2017-05-31 23:21:28]

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-06-01 01:32:54]

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

[commit-bot: I haz the power, 2017-06-01 01:32:56]

Dry run: This issue passed the CQ dry run.

[Dmitry Titov, 2017-06-06 04:42:09]

https://codereview.chromium.org/2912213002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/serializers/MarkupAccumulator.h
(right):

https://codereview.chromium.org/2912213002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/serializers/MarkupAccumulator.h:81:
virtual Node* GetAuxiliaryDOMTree(const Element&, Element**) const;
I think it would be clearer to return a std::pair<Node*, Element*>here. 
Also, add comment when each of them can be null.
Also, it'd be good to mention that it creates a new <template> element and
returns it. Maybe change "enclosing element" references to read as "template
element"?

https://codereview.chromium.org/2912213002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/LocalFrameClientImpl.cpp (right):

https://codereview.chromium.org/2912213002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/LocalFrameClientImpl.cpp:230:
parent.removeChild(template);
Just to be on a safe side, lets check parent for null...

[jianli, 2017-06-07 00:53:42]

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

[jianli, 2017-06-07 00:53:42]

https://codereview.chromium.org/2912213002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/serializers/MarkupAccumulator.h
(right):

https://codereview.chromium.org/2912213002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/serializers/MarkupAccumulator.h:81:
virtual Node* GetAuxiliaryDOMTree(const Element&, Element**) const;
On 2017/06/06 04:42:09, Dmitry Titov wrote:
> I think it would be clearer to return a std::pair<Node*, Element*>here. 
> Also, add comment when each of them can be null.
> Also, it'd be good to mention that it creates a new <template> element and
> returns it. Maybe change "enclosing element" references to read as "template
> element"?

Done.

https://codereview.chromium.org/2912213002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/LocalFrameClientImpl.cpp (right):

https://codereview.chromium.org/2912213002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/LocalFrameClientImpl.cpp:230:
parent.removeChild(template);
On 2017/06/06 04:42:09, Dmitry Titov wrote:
> Just to be on a safe side, lets check parent for null...

Done.

[commit-bot: I haz the power, 2017-06-07 00:54:22]

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-06-07 02:46:34]

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

[commit-bot: I haz the power, 2017-06-07 02:46:35]

Dry run: This issue passed the CQ dry run.

[Dmitry Titov, 2017-06-07 17:40:30]

Did you mean to upload a new version?

[jianli, 2017-06-07 19:07:20]

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

[jianli, 2017-06-07 19:07:34]

Forgot to upload a new version. Done.

On Wed, Jun 7, 2017 at 10:40 AM, <dimich@chromium.org> wrote:

> Did you mean to upload a new version?
>
> https://codereview.chromium.org/2912213002/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[jianli, 2017-06-07 19:07:35]

Forgot to upload a new version. Done.

On Wed, Jun 7, 2017 at 10:40 AM, <dimich@chromium.org> wrote:

> Did you mean to upload a new version?
>
> https://codereview.chromium.org/2912213002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2017-06-07 19:08:10]

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-06-07 19:23:53]

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

[commit-bot: I haz the power, 2017-06-07 19:23:54]

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_...)

[jianli, 2017-06-13 00:45:06]

Ping. Thanks.

[Dmitry Titov, 2017-06-13 04:40:23]

LGTM.

sorry for delay!

[Mike West, 2017-06-14 07:03:59]

LGTM % one question.

https://codereview.chromium.org/2912213002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebFrameSerializer.cpp (right):

https://codereview.chromium.org/2912213002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebFrameSerializer.cpp:219: attribute.LocalName()
== kShadowModeAttributeName &&
Should we be stripping `kShadowDelegatesFocusAttributeName` as well?

[jianli, 2017-06-14 19:06:08]

https://codereview.chromium.org/2912213002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebFrameSerializer.cpp (right):

https://codereview.chromium.org/2912213002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebFrameSerializer.cpp:219: attribute.LocalName()
== kShadowModeAttributeName &&
On 2017/06/14 07:03:59, Mike West wrote:
> Should we be stripping `kShadowDelegatesFocusAttributeName` as well?

kShadowDelegatesFocusAttributeName can't take any effect by itself. So I don't
want to add additional code for this.

[Dmitry Titov, 2017-06-14 19:33:11]

https://codereview.chromium.org/2912213002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebFrameSerializer.cpp (right):

https://codereview.chromium.org/2912213002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebFrameSerializer.cpp:219: attribute.LocalName()
== kShadowModeAttributeName &&
On 2017/06/14 19:06:08, jianli wrote:
> On 2017/06/14 07:03:59, Mike West wrote:
> > Should we be stripping `kShadowDelegatesFocusAttributeName` as well?
> 
> kShadowDelegatesFocusAttributeName can't take any effect by itself. So I don't
> want to add additional code for this.

Hm, stripping all attributes that feature adds might be a bit overkill, but it
can simplify the analysis of code down the road... Instead of figuring out which
attributes cause JS to run and which don't, it's easier to just know all of them
are stripped. WDYT?

[jianli, 2017-06-14 21:48:06]

https://codereview.chromium.org/2912213002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebFrameSerializer.cpp (right):

https://codereview.chromium.org/2912213002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebFrameSerializer.cpp:219: attribute.LocalName()
== kShadowModeAttributeName &&
On 2017/06/14 19:33:10, Dmitry Titov wrote:
> On 2017/06/14 19:06:08, jianli wrote:
> > On 2017/06/14 07:03:59, Mike West wrote:
> > > Should we be stripping `kShadowDelegatesFocusAttributeName` as well?
> > 
> > kShadowDelegatesFocusAttributeName can't take any effect by itself. So I
don't
> > want to add additional code for this.
> 
> Hm, stripping all attributes that feature adds might be a bit overkill, but it
> can simplify the analysis of code down the road... Instead of figuring out
which
> attributes cause JS to run and which don't, it's easier to just know all of
them
> are stripped. WDYT?

Sounds reasonable. I am going to remove this attribute too.

[jianli, 2017-06-14 21:48:46]

The CQ bit was checked by jianli@chromium.org

[jianli, 2017-06-14 21:48:47]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org,
dimich@chromium.org
Link to the patchset: https://codereview.chromium.org/2912213002/#ps140001
(title: "Strip anoter shadow attribute")

[commit-bot: I haz the power, 2017-06-14 21:49:08]

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-06-15 00:11:20]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1497476926952990,
"parent_rev": "0607b7981ab9de1b2bbfc1de6a9ae2c2fb70786b", "commit_rev":
"05d415724da48cbeba542558a3ce5689ec0fee93"}

[commit-bot: I haz the power, 2017-06-15 00:11:32]

Description was changed from

==========
Support serializing shadow DOM to MHTML

The content of shadow DOM tree will be serialized and put within a template
element that appends to the children of the shadow host element. The template
element contains a special attribute to denote the mode of the shadow dom tree.

When MHTML page is being loaded, the shadow DOM tree will be recreated by the
injected script to convert the templates to the shadow DOM trees.

Please see the bug for the link to the design doc.

BUG=695738
TEST=new tests added
==========

to

==========
Support serializing shadow DOM to MHTML

The content of shadow DOM tree will be serialized and put within a template
element that appends to the children of the shadow host element. The template
element contains a special attribute to denote the mode of the shadow dom tree.

When MHTML page is being loaded, the shadow DOM tree will be recreated by the
injected script to convert the templates to the shadow DOM trees.

Please see the bug for the link to the design doc.

BUG=695738
TEST=new tests added

Review-Url: https://codereview.chromium.org/2912213002
Cr-Commit-Position: refs/heads/master@{#479550}
Committed:
https://chromium.googlesource.com/chromium/src/+/05d415724da48cbeba542558a3ce...
==========

[commit-bot: I haz the power, 2017-06-15 00:11:33]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/05d415724da48cbeba542558a3ce...