Add <link rel=preload> onload support for scripts and styles

Add <link rel=preload> onload support for scripts and styles

This CL relands https://codereview.chromium.org/1577073005/ which adds onload support for <link rel=preload> for scripts and styles.
Support for other resource types will be added in a followup CL.

BUG=552289
Committed: https://crrev.com/dcbdcdb41ad3b1dbb0426b3092592b2c451ad098
Cr-Commit-Position: refs/heads/master@{#370456}

[Yoav Weiss, 2016-01-16 06:19:53]

Description was changed from

==========
Add <link rel=preload> onload support for scripts and styles

This CL adds onload support for <link rel=preload> for scripts and styles.
Support for other resource types will be added in a followup CL.

BUG=552289
==========

to

==========
Add <link rel=preload> onload support for scripts and styles

This CL relands https://codereview.chromium.org/1577073005/ which adds onload
support for <link rel=preload> for scripts and styles.
Support for other resource types will be added in a followup CL.

BUG=552289
==========

[Yoav Weiss, 2016-01-16 06:19:53]

yoav@yoav.ws changed reviewers:
+ japhet@chromium.org

[Yoav Weiss, 2016-01-16 06:21:30]

Compile issue(s) caused https://codereview.chromium.org/1577073005/ to break the
oilpan build. This is the same patch, hopefully fixing these issues.

Nate - can you take a look? :)

[Yoav Weiss, 2016-01-16 20:57:39]

On 2016/01/16 06:21:30, Yoav Weiss wrote:
> Compile issue(s) caused https://codereview.chromium.org/1577073005/ to break
the
> oilpan build. This is the same patch, hopefully fixing these issues.
> 
> Nate - can you take a look? :)

On second thought, might be better to wait for
https://codereview.chromium.org/1569273004/ to land before this one, as this may
change things for oilpan.

[sof, 2016-01-20 12:22:53]

sigbjornf@opera.com changed reviewers:
+ sigbjornf@opera.com

[sof, 2016-01-20 12:22:53]

https://codereview.chromium.org/1586563014/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/loader/LinkLoader.cpp (right):

https://codereview.chromium.org/1586563014/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/LinkLoader.cpp:330:
visitor->trace(m_linkPreloadResourceClient);
move up a line.

https://codereview.chromium.org/1586563014/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/loader/LinkLoader.h (right):

https://codereview.chromium.org/1586563014/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/LinkLoader.h:96:
OwnPtrWillBeMember<LinkPreloadResourceClient> m_linkPreloadResourceClient;
This is another reason why LinkPreloadResourceClient needs to be a GC mixin --
so that you can correctly trace it from this class.

https://codereview.chromium.org/1586563014/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/loader/LinkPreloadResourceClients.h (right):

https://codereview.chromium.org/1586563014/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/LinkPreloadResourceClients.h:19: class
LinkPreloadResourceClient {
Since this contains a heap reference, you need to trace it. The way we normally
do that for these client base classes is to turn them into GC mixins --


  class LinkPreloadResourceClient : public WillBeGarbageCollectedMixin {
  public:
    ...
      DEFINE_INLINE_VIRTUAL_TRACE()
      {
          visitor->trace(m_loader);
      }
  }

(I think you may have to define that trace in the .cpp file though, as
LinkLoader's definition is not in scope...)

+ have each class that derive from LinkPreloadResourceClient delegate to
LinkPreloadResourceClient::trace(visitor) also.

https://codereview.chromium.org/1586563014/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/LinkPreloadResourceClients.h:33:
LinkLoader* m_loader;
RawPtrWillBeMember<LinkLoader>

[Yoav Weiss, 2016-01-20 13:14:02]

https://codereview.chromium.org/1586563014/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/loader/LinkLoader.cpp (right):

https://codereview.chromium.org/1586563014/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/LinkLoader.cpp:330:
visitor->trace(m_linkPreloadResourceClient);
On 2016/01/20 12:22:53, sof wrote:
> move up a line.

moved

https://codereview.chromium.org/1586563014/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/loader/LinkPreloadResourceClients.h (right):

https://codereview.chromium.org/1586563014/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/LinkPreloadResourceClients.h:19: class
LinkPreloadResourceClient {
On 2016/01/20 12:22:53, sof wrote:
> Since this contains a heap reference, you need to trace it. The way we
normally
> do that for these client base classes is to turn them into GC mixins --
> 
> 
>   class LinkPreloadResourceClient : public WillBeGarbageCollectedMixin {
>   public:
>     ...
>       DEFINE_INLINE_VIRTUAL_TRACE()
>       {
>           visitor->trace(m_loader);
>       }
>   }
> 
> (I think you may have to define that trace in the .cpp file though, as
> LinkLoader's definition is not in scope...)
> 
> + have each class that derive from LinkPreloadResourceClient delegate to
> LinkPreloadResourceClient::trace(visitor) also.

Had to define it as NoBaseWillBeGarbageCollectedFinalized, as otherwise, I got
'static_assert failed "only garbage collected objects can have garbage collected
mixins"'

https://codereview.chromium.org/1586563014/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/LinkPreloadResourceClients.h:33:
LinkLoader* m_loader;
On 2016/01/20 12:22:53, sof wrote:
> RawPtrWillBeMember<LinkLoader>

done

[sof, 2016-01-20 13:16:15]

https://codereview.chromium.org/1586563014/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/loader/LinkPreloadResourceClients.h (right):

https://codereview.chromium.org/1586563014/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/loader/LinkPreloadResourceClients.h:23: void
triggerEvents(const Resource*) const;
(odd-looking const method)

https://codereview.chromium.org/1586563014/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/loader/LinkPreloadResourceClients.h:41: class
LinkPreloadScriptResourceClient: public ResourceOwner<ScriptResource,
ScriptResourceClient>, public LinkPreloadResourceClient {
LinkPreloadResourceClient needs to be the leftmost (GC) base, so switch them
around.

https://codereview.chromium.org/1586563014/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/loader/LinkPreloadResourceClients.h:71: class
LinkPreloadStyleResourceClient: public ResourceOwner<CSSStyleSheetResource,
StyleSheetResourceClient>, public LinkPreloadResourceClient {
ditto

[sof, 2016-01-20 13:22:41]

https://codereview.chromium.org/1586563014/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/loader/LinkPreloadResourceClients.h (right):

https://codereview.chromium.org/1586563014/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/loader/LinkPreloadResourceClients.h:1: //
Copyright 2017 The Chromium Authors. All rights reserved.
off by one :)

https://codereview.chromium.org/1586563014/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/loader/LinkPreloadResourceClients.h (right):

https://codereview.chromium.org/1586563014/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/loader/LinkPreloadResourceClients.h:42:
WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(LinkPreloadScriptResourceClient);
Add USING_FAST_MALLOC_WILL_BE_REMOVED(..) on this class (and the one below)?

[Yoav Weiss, 2016-01-20 13:28:05]

https://codereview.chromium.org/1586563014/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/loader/LinkPreloadResourceClients.h (right):

https://codereview.chromium.org/1586563014/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/loader/LinkPreloadResourceClients.h:1: //
Copyright 2017 The Chromium Authors. All rights reserved.
On 2016/01/20 13:22:41, sof wrote:
> off by one :)

thinking ahead :)

https://codereview.chromium.org/1586563014/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/loader/LinkPreloadResourceClients.h:23: void
triggerEvents(const Resource*) const;
On 2016/01/20 13:16:15, sof wrote:
> (odd-looking const method)

removed the const part as it's not necessary.

https://codereview.chromium.org/1586563014/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/loader/LinkPreloadResourceClients.h:41: class
LinkPreloadScriptResourceClient: public ResourceOwner<ScriptResource,
ScriptResourceClient>, public LinkPreloadResourceClient {
On 2016/01/20 13:16:15, sof wrote:
> LinkPreloadResourceClient needs to be the leftmost (GC) base, so switch them
> around.

switched order

https://codereview.chromium.org/1586563014/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/loader/LinkPreloadResourceClients.h:71: class
LinkPreloadStyleResourceClient: public ResourceOwner<CSSStyleSheetResource,
StyleSheetResourceClient>, public LinkPreloadResourceClient {
On 2016/01/20 13:16:15, sof wrote:
> ditto

switched order

https://codereview.chromium.org/1586563014/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/loader/LinkPreloadResourceClients.h (right):

https://codereview.chromium.org/1586563014/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/loader/LinkPreloadResourceClients.h:42:
WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(LinkPreloadScriptResourceClient);
On 2016/01/20 13:22:41, sof wrote:
> Add USING_FAST_MALLOC_WILL_BE_REMOVED(..) on this class (and the one below)?

Added

[sof, 2016-01-20 13:40:03]

(oilpan) lgtm, but japhet@ will hopefully also have a look.

[Yoav Weiss, 2016-01-20 13:44:44]

On 2016/01/20 13:40:03, sof wrote:
> (oilpan) lgtm, but japhet@ will hopefully also have a look.

japhet@ - what say you? This is the same change as
https://codereview.chromium.org/1577073005/ only without breaking oilpan

[Nate Chapin, 2016-01-20 18:51:22]

lgtm

[Yoav Weiss, 2016-01-20 18:51:57]

The CQ bit was checked by yoav@yoav.ws

[commit-bot: I haz the power, 2016-01-20 18:53:25]

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

[commit-bot: I haz the power, 2016-01-20 19:58:09]

Description was changed from

==========
Add <link rel=preload> onload support for scripts and styles

This CL relands https://codereview.chromium.org/1577073005/ which adds onload
support for <link rel=preload> for scripts and styles.
Support for other resource types will be added in a followup CL.

BUG=552289
==========

to

==========
Add <link rel=preload> onload support for scripts and styles

This CL relands https://codereview.chromium.org/1577073005/ which adds onload
support for <link rel=preload> for scripts and styles.
Support for other resource types will be added in a followup CL.

BUG=552289
==========

[commit-bot: I haz the power, 2016-01-20 19:58:10]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-01-20 19:59:56]

Description was changed from

==========
Add <link rel=preload> onload support for scripts and styles

This CL relands https://codereview.chromium.org/1577073005/ which adds onload
support for <link rel=preload> for scripts and styles.
Support for other resource types will be added in a followup CL.

BUG=552289
==========

to

==========
Add <link rel=preload> onload support for scripts and styles

This CL relands https://codereview.chromium.org/1577073005/ which adds onload
support for <link rel=preload> for scripts and styles.
Support for other resource types will be added in a followup CL.

BUG=552289

Committed: https://crrev.com/dcbdcdb41ad3b1dbb0426b3092592b2c451ad098
Cr-Commit-Position: refs/heads/master@{#370456}
==========

[commit-bot: I haz the power, 2016-01-20 19:59:57]

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