DevTools: fall back to fetching stylesheet source from the disk cache

DevTools: fall back to fetching stylesheet source from the disk cache

BUG=292613
R=vsevik@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=158185

[pfeldman, 2013-09-18 14:29:10]

This is a straw man proposal on the short-term fix for resource-less
stylesheets.

[apavlov, 2013-09-18 15:36:44]

https://codereview.chromium.org/24151005/diff/11001/Source/core/inspector/Ins...
File Source/core/inspector/InspectorCSSAgent.cpp (right):

https://codereview.chromium.org/24151005/diff/11001/Source/core/inspector/Ins...
Source/core/inspector/InspectorCSSAgent.cpp:281: explicit
EnableResourceClient(InspectorCSSAgent* cssAgent, const
Vector<InspectorStyleSheet*>& styleSheets, PassRefPtr<EnableCallback> callback)
"explicit" is not required (and disproved) for ctors with multiple arguments

https://codereview.chromium.org/24151005/diff/11001/Source/core/inspector/Ins...
Source/core/inspector/InspectorCSSAgent.cpp:844: // Re-issue stylesheet requets
for resources that are no longer in memory cache.
requets->requests

I also believe this code should be extracted into a separate method.

https://codereview.chromium.org/24151005/diff/11001/Source/core/inspector/Ins...
Source/core/inspector/InspectorCSSAgent.cpp:845: Vector<InspectorStyleSheet*>
styleSheestToFetch;
typo: styleSheetsToFetch

https://codereview.chromium.org/24151005/diff/11001/Source/core/inspector/Ins...
Source/core/inspector/InspectorCSSAgent.cpp:853: // We are only interested in
the stylesheets that are already loaded. Otherwise we pick all media
stylesheets.
This comment needs refinement. Did you mean that stylesheets with non-matching
media shall be requested? We have identified the fact that they are loaded
always, but do not block page tree construction.

So, if we switch to the CSS media emulation mode, we'll have matching rules from
those stylesheets, but their source data will not be available?

https://codereview.chromium.org/24151005/diff/11001/Source/core/inspector/Ins...
File Source/core/inspector/InspectorResourceAgent.cpp (right):

https://codereview.chromium.org/24151005/diff/11001/Source/core/inspector/Ins...
Source/core/inspector/InspectorResourceAgent.cpp:715: 
extraneous blank line

[apavlov, 2013-09-23 12:20:38]

https://codereview.chromium.org/24151005/diff/11001/Source/core/inspector/Ins...
File Source/core/inspector/InspectorCSSAgent.cpp (right):

https://codereview.chromium.org/24151005/diff/11001/Source/core/inspector/Ins...
Source/core/inspector/InspectorCSSAgent.cpp:279: class
InspectorCSSAgent::EnableResourceClient : public StyleSheetResourceClient {
Wouldn't it be a good idea to declare this class in the header, as we usually do
with nested classes?

[vsevik, 2013-09-23 12:35:49]

https://chromiumcodereview.appspot.com/24151005/diff/11001/Source/core/inspec...
File Source/core/inspector/InspectorCSSAgent.cpp (right):

https://chromiumcodereview.appspot.com/24151005/diff/11001/Source/core/inspec...
Source/core/inspector/InspectorCSSAgent.cpp:289: Document* document =
styleSheet->ownerDocument();
null check?

https://chromiumcodereview.appspot.com/24151005/diff/11001/Source/core/inspec...
Source/core/inspector/InspectorCSSAgent.cpp:864: if (!prpCallback ||
styleSheestToFetch.isEmpty()) {
I don't think checking prpCallback for null makes sense.

https://chromiumcodereview.appspot.com/24151005/diff/11001/Source/core/inspec...
File Source/core/inspector/InspectorResourceAgent.cpp (right):

https://chromiumcodereview.appspot.com/24151005/diff/11001/Source/core/inspec...
Source/core/inspector/InspectorResourceAgent.cpp:290: if (initiatorInfo.name ==
FetchInitiatorTypeNames::inspector)
I don't think we should hide these requests.

[pfeldman, 2013-09-23 12:45:41]

https://chromiumcodereview.appspot.com/24151005/diff/11001/Source/core/inspec...
File Source/core/inspector/InspectorCSSAgent.cpp (right):

https://chromiumcodereview.appspot.com/24151005/diff/11001/Source/core/inspec...
Source/core/inspector/InspectorCSSAgent.cpp:279: class
InspectorCSSAgent::EnableResourceClient : public StyleSheetResourceClient {
On 2013/09/23 12:20:38, apavlov wrote:
> Wouldn't it be a good idea to declare this class in the header, as we usually
do
> with nested classes?

Done.

https://chromiumcodereview.appspot.com/24151005/diff/11001/Source/core/inspec...
Source/core/inspector/InspectorCSSAgent.cpp:281: explicit
EnableResourceClient(InspectorCSSAgent* cssAgent, const
Vector<InspectorStyleSheet*>& styleSheets, PassRefPtr<EnableCallback> callback)
On 2013/09/18 15:36:44, apavlov wrote:
> "explicit" is not required (and disproved) for ctors with multiple arguments

Done.

https://chromiumcodereview.appspot.com/24151005/diff/11001/Source/core/inspec...
Source/core/inspector/InspectorCSSAgent.cpp:289: Document* document =
styleSheet->ownerDocument();
On 2013/09/23 12:35:49, vsevik wrote:
> null check?

Done.

https://chromiumcodereview.appspot.com/24151005/diff/11001/Source/core/inspec...
Source/core/inspector/InspectorCSSAgent.cpp:844: // Re-issue stylesheet requets
for resources that are no longer in memory cache.
On 2013/09/18 15:36:44, apavlov wrote:
> requets->requests
> 
> I also believe this code should be extracted into a separate method.

It is only 30 lines of code, why?

https://chromiumcodereview.appspot.com/24151005/diff/11001/Source/core/inspec...
Source/core/inspector/InspectorCSSAgent.cpp:845: Vector<InspectorStyleSheet*>
styleSheestToFetch;
On 2013/09/18 15:36:44, apavlov wrote:
> typo: styleSheetsToFetch

Done.

https://chromiumcodereview.appspot.com/24151005/diff/11001/Source/core/inspec...
Source/core/inspector/InspectorCSSAgent.cpp:853: // We are only interested in
the stylesheets that are already loaded. Otherwise we pick all media
stylesheets.
Removed comment.

https://chromiumcodereview.appspot.com/24151005/diff/11001/Source/core/inspec...
Source/core/inspector/InspectorCSSAgent.cpp:864: if (!prpCallback ||
styleSheestToFetch.isEmpty()) {
On 2013/09/23 12:35:49, vsevik wrote:
> I don't think checking prpCallback for null makes sense.

It is 0 when invoked from restore.

https://chromiumcodereview.appspot.com/24151005/diff/11001/Source/core/inspec...
File Source/core/inspector/InspectorResourceAgent.cpp (right):

https://chromiumcodereview.appspot.com/24151005/diff/11001/Source/core/inspec...
Source/core/inspector/InspectorResourceAgent.cpp:290: if (initiatorInfo.name ==
FetchInitiatorTypeNames::inspector)
On 2013/09/23 12:35:49, vsevik wrote:
> I don't think we should hide these requests.

Lets start like  this and see if it leads to any issues?

https://chromiumcodereview.appspot.com/24151005/diff/11001/Source/core/inspec...
Source/core/inspector/InspectorResourceAgent.cpp:715: 
On 2013/09/18 15:36:44, apavlov wrote:
> extraneous blank line

Done.

[vsevik, 2013-09-23 12:52:42]

lgtm

[commit-bot: I haz the power, 2013-09-23 12:58:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pfeldman@chromium.org/24151005/19001

[commit-bot: I haz the power, 2013-09-23 13:53:00]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_blink_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_blink_...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[pfeldman, 2013-09-23 14:35:05]

Committed patchset #2 manually as r158185 (presubmit successful).