DevTools: Load document (html) content from disk cache in page agent enabling.

DevTools: Load document (html) content from disk cache in page agent enabling.

BUG=354447
R=pfeldman@chromium.org

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

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

[vsevik, 2014-05-07 14:57:45]

PTAL

[pfeldman, 2014-05-20 19:50:48]

https://codereview.chromium.org/267393003/diff/110001/Source/core/inspector/I...
File Source/core/inspector/InspectorController.cpp (right):

https://codereview.chromium.org/267393003/diff/110001/Source/core/inspector/I...
Source/core/inspector/InspectorController.cpp:255: for
(Vector<String>::const_iterator it = m_messages.begin(); it != m_messages.end();
++it)
You want to iterate a copy just in case.

https://codereview.chromium.org/267393003/diff/110001/Source/core/inspector/I...
File Source/core/inspector/InspectorController.h (right):

https://codereview.chromium.org/267393003/diff/110001/Source/core/inspector/I...
Source/core/inspector/InspectorController.h:93: void
connectFrontend(InspectorFrontendChannel*, bool isReusing = false);
There are no public callers with isReusing defined, please use private method.

https://codereview.chromium.org/267393003/diff/110001/Source/core/inspector/I...
Source/core/inspector/InspectorController.h:168: Vector<String> m_messages;
what messages?

https://codereview.chromium.org/267393003/diff/110001/Source/core/inspector/I...
File Source/core/inspector/InspectorResourceContentLoader.cpp (right):

https://codereview.chromium.org/267393003/diff/110001/Source/core/inspector/I...
Source/core/inspector/InspectorResourceContentLoader.cpp:38: if
(!document->frame() ||
!document->frame()->loader().documentResourceRequestFromCurrentHistoryItemForInspector(ReturnCacheDataDontLoad,
&resourceRequest)) {
It is unclear why we use this for document, but not for other types of
resources.

https://codereview.chromium.org/267393003/diff/110001/Source/core/inspector/I...
File Source/core/inspector/InspectorResourceContentLoader.h (right):

https://codereview.chromium.org/267393003/diff/110001/Source/core/inspector/I...
Source/core/inspector/InspectorResourceContentLoader.h:37: int
m_pendingResourcesCount;
m_pendingResourceCount

https://codereview.chromium.org/267393003/diff/110001/Source/core/loader/Fram...
File Source/core/loader/FrameLoader.h (right):

https://codereview.chromium.org/267393003/diff/110001/Source/core/loader/Fram...
Source/core/loader/FrameLoader.h:193: bool
documentResourceRequestFromCurrentHistoryItemForInspector(ResourceRequestCachePolicy,
ResourceRequest*);
The name is long, yet unclear.

[vsevik, 2014-05-21 11:25:24]

>
https://codereview.chromium.org/267393003/diff/110001/Source/core/inspector/I...
> Source/core/inspector/InspectorController.cpp:255: for
> (Vector<String>::const_iterator it = m_messages.begin(); it !=
m_messages.end();
> ++it)
> You want to iterate a copy just in case.
Done.

>
https://codereview.chromium.org/267393003/diff/110001/Source/core/inspector/I...
> Source/core/inspector/InspectorController.h:93: void
> connectFrontend(InspectorFrontendChannel*, bool isReusing = false);
> There are no public callers with isReusing defined, please use private method.
Done.

>
https://codereview.chromium.org/267393003/diff/110001/Source/core/inspector/I...
> Source/core/inspector/InspectorController.h:168: Vector<String> m_messages;
> what messages?
Done.

>
https://codereview.chromium.org/267393003/diff/110001/Source/core/inspector/I...
> Source/core/inspector/InspectorResourceContentLoader.cpp:38: if
> (!document->frame() ||
>
!document->frame()->loader().documentResourceRequestFromCurrentHistoryItemForInspector(ReturnCacheDataDontLoad,
> &resourceRequest)) {
> It is unclear why we use this for document, but not for other types of
> resources.
What do you mean?

>
https://codereview.chromium.org/267393003/diff/110001/Source/core/inspector/I...
> Source/core/inspector/InspectorResourceContentLoader.h:37: int
> m_pendingResourcesCount;
> m_pendingResourceCount
Done.

>
https://codereview.chromium.org/267393003/diff/110001/Source/core/loader/Fram...
> Source/core/loader/FrameLoader.h:193: bool
>
documentResourceRequestFromCurrentHistoryItemForInspector(ResourceRequestCachePolicy,
> ResourceRequest*);
> The name is long, yet unclear.
Done.

[vsevik, 2014-05-21 11:25:32]

PTAL

[pfeldman, 2014-05-21 14:25:10]

lgtm.

[vsevik, 2014-05-21 14:27:36]

The CQ bit was checked by vsevik@chromium.org

[commit-bot: I haz the power, 2014-05-21 17:51:15]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vsevik@chromium.org/267393003/130001

[vsevik, 2014-05-22 14:49:34]

Committed patchset #10 manually as r174557 (presubmit successful).

[miichefurr, 2014-05-23 09:25:17]

On 2014/05/22 14:49:34, vsevik wrote:
> Committed patchset #10 manually as r174557 (presubmit successful

Hi.

I know this may be a silly question, but how do I install this fix ?

Regards
Michael

[vsevik, 2014-06-13 09:56:51]

pfeldman@, could you please take another look?

This patch uses a new approach to collecting resources content.
When page agent is enabled an InspectorResourceContentLoader object is created.
CSS.enable() and Page.getResourceContent() (this one is now async) protocol
methods add theirselves as a listener to it to wait for resource content loading
completion.
This patch makes CSS domain implicitly dependent on Page agent being enabled,
though it would still work normally in most cases.

[pfeldman, 2014-06-16 08:42:19]

lgtm

[vsevik, 2014-06-16 08:55:39]

The CQ bit was checked by vsevik@chromium.org

[commit-bot: I haz the power, 2014-06-16 08:56:44]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vsevik@chromium.org/267393003/270001

[commit-bot: I haz the power, 2014-06-16 10:16:04]

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

[commit-bot: I haz the power, 2014-06-16 10:16:06]

Try jobs failed on following builders:
  mac_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_compile_dbg/bu...)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/11477)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/17124)

[vsevik, 2014-06-16 11:17:09]

The CQ bit was checked by vsevik@chromium.org

[commit-bot: I haz the power, 2014-06-16 11:18:09]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vsevik@chromium.org/267393003/290001

[commit-bot: I haz the power, 2014-06-16 12:50:41]

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

[commit-bot: I haz the power, 2014-06-16 12:50:43]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)

[vsevik, 2014-06-16 13:16:41]

The CQ bit was checked by vsevik@chromium.org

[commit-bot: I haz the power, 2014-06-16 13:16:49]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vsevik@chromium.org/267393003/310001

[commit-bot: I haz the power, 2014-06-16 15:39:10]

Change committed as 176213

[Dirk Pranke, 2014-06-16 22:06:54]

This change appears to have made
inspector-protocol/stylesheet-tracking-restart.html substantially flakier.

I've noted this w/ a suppression in TestExpectations (r176245).

[vsevik, 2014-06-17 07:58:10]

On 2014/06/16 22:06:54, Dirk Pranke wrote:
> This change appears to have made
> inspector-protocol/stylesheet-tracking-restart.html substantially flakier.
> 
> I've noted this w/ a suppression in TestExpectations (r176245).

Thanks, I have fixed it in https://codereview.chromium.org/339133002/