[DevTools] Move style sheet content storage from css agent to resource loader.

[DevTools] Move style sheet content storage from css agent to resource loader.

This is required to destroy agents on detach as a part of multiclient work.

BUG=590878
TBR=pfeldman@chromium.org

Committed: https://crrev.com/139c34259dd888542d53715f68d5abb8f1777b44
Cr-Commit-Position: refs/heads/master@{#387522}

[dgozman, 2016-04-14 03:34:06]

dgozman@chromium.org changed reviewers:
+ caseq@chromium.org

[dgozman, 2016-04-14 03:34:06]

Take a look please.

[caseq, 2016-04-14 19:20:40]

https://codereview.chromium.org/1888673002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/inspector/InspectorResourceContentLoader.h
(right):

https://codereview.chromium.org/1888673002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/InspectorResourceContentLoader.h:57:
HashMap<int, String> m_styleElementContents;
It looks like this couple is not particularly related to the logic of inspector
resource loader...

https://codereview.chromium.org/1888673002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/inspector/InspectorStyleSheet.h (right):

https://codereview.chromium.org/1888673002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/InspectorStyleSheet.h:123: static
InspectorStyleSheet* create(InspectorResourceAgent*, CSSStyleSheet*
pageStyleSheet, const String& origin, const String& documentURL,
InspectorStyleSheetBase::Listener*, InspectorResourceContentLoader*);
... and putting it there introduces new dependencies on the resource content
loader. So, perhaps, just put it into a class of its own?

[dgozman, 2016-04-15 00:10:28]

Please take another look.

[caseq, 2016-04-15 00:31:54]

lgtm

https://codereview.chromium.org/1888673002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/InspectorResourceContainer.cpp
(right):

https://codereview.chromium.org/1888673002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorResourceContainer.cpp:27: if
(frame == m_inspectedFrames->root()) {
nit:
if (frame != m_inspectedFrames->root())
    return;

https://codereview.chromium.org/1888673002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorResourceContainer.cpp:40: if
(!m_styleSheetContents.contains(url))
please consider
*content = m_styleSheetContents.get(url);
return !content->isNull()

https://codereview.chromium.org/1888673002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorResourceContainer.cpp:53: if
(!m_styleElementContents.contains(backendNodeId))
ditto.

https://codereview.chromium.org/1888673002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/InspectorResourceContainer.h
(right):

https://codereview.chromium.org/1888673002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorResourceContainer.h:24: static
InspectorResourceContainer* create(InspectedFrames* inspectedFrames)
nuke and make constructor public?

[dgozman, 2016-04-15 00:48:44]

https://codereview.chromium.org/1888673002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/InspectorResourceContainer.cpp
(right):

https://codereview.chromium.org/1888673002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorResourceContainer.cpp:27: if
(frame == m_inspectedFrames->root()) {
On 2016/04/15 00:31:54, caseq wrote:
> nit:
> if (frame != m_inspectedFrames->root())
>     return;
>    

Done.

https://codereview.chromium.org/1888673002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorResourceContainer.cpp:40: if
(!m_styleSheetContents.contains(url))
On 2016/04/15 00:31:54, caseq wrote:
> please consider
> *content = m_styleSheetContents.get(url);
> return !content->isNull()

Acknowledged.

https://codereview.chromium.org/1888673002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/InspectorResourceContainer.h
(right):

https://codereview.chromium.org/1888673002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/InspectorResourceContainer.h:24: static
InspectorResourceContainer* create(InspectedFrames* inspectedFrames)
On 2016/04/15 00:31:54, caseq wrote:
> nuke and make constructor public?

Done.

[dgozman, 2016-04-15 00:48:55]

The CQ bit was checked by dgozman@chromium.org

[dgozman, 2016-04-15 00:48:56]

The patchset sent to the CQ was uploaded after l-g-t-m from caseq@chromium.org
Link to the patchset: https://codereview.chromium.org/1888673002/#ps40001
(title: "nits")

[commit-bot: I haz the power, 2016-04-15 00:49:35]

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

[commit-bot: I haz the power, 2016-04-15 00:57:08]

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

[commit-bot: I haz the power, 2016-04-15 00:57:09]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[dgozman, 2016-04-15 01:44:39]

Description was changed from

==========
[DevTools] Move style sheet content storage from css agent to resource loader.

This is required to destroy agents on detach as a part of multiclient work.

BUG=590878
==========

to

==========
[DevTools] Move style sheet content storage from css agent to resource loader.

This is required to destroy agents on detach as a part of multiclient work.

BUG=590878
TBR=pfeldman@chromium.org
==========

[dgozman, 2016-04-15 01:44:53]

TBR'ing pfeldman for core.gypi

[dgozman, 2016-04-15 01:44:58]

The CQ bit was checked by dgozman@chromium.org

[commit-bot: I haz the power, 2016-04-15 01:45:27]

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

[commit-bot: I haz the power, 2016-04-15 02:22:18]

Description was changed from

==========
[DevTools] Move style sheet content storage from css agent to resource loader.

This is required to destroy agents on detach as a part of multiclient work.

BUG=590878
TBR=pfeldman@chromium.org
==========

to

==========
[DevTools] Move style sheet content storage from css agent to resource loader.

This is required to destroy agents on detach as a part of multiclient work.

BUG=590878
TBR=pfeldman@chromium.org
==========

[commit-bot: I haz the power, 2016-04-15 02:22:19]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-15 02:25:19]

Description was changed from

==========
[DevTools] Move style sheet content storage from css agent to resource loader.

This is required to destroy agents on detach as a part of multiclient work.

BUG=590878
TBR=pfeldman@chromium.org
==========

to

==========
[DevTools] Move style sheet content storage from css agent to resource loader.

This is required to destroy agents on detach as a part of multiclient work.

BUG=590878
TBR=pfeldman@chromium.org

Committed: https://crrev.com/139c34259dd888542d53715f68d5abb8f1777b44
Cr-Commit-Position: refs/heads/master@{#387522}
==========

[commit-bot: I haz the power, 2016-04-15 02:25:21]

Patchset 3 (id:??) landed as
https://crrev.com/139c34259dd888542d53715f68d5abb8f1777b44
Cr-Commit-Position: refs/heads/master@{#387522}