Bug 103937 – [chromium] fix showFPScounter and showPaintRects not toggleable after page change

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[chromium] fix showFPScounter and showPaintRects not toggleable after page change

Created attachment 177348 [details]
Patch

This bug was caused by a recent change in the way the compositor handles commandline settings and webinspector settings for showFPSCounter and showPaintRects. These settings are now seperate and get ORed together. 

Right now the showFPSCounter and showPaintRects values are updated in the WebSettings. The problem is that these values then become the commandline setting when the page is refreshed with the WebInspector opened. So they can no longer be toggled.

In summary, this change avoids updating the values in the WebSettings and instead reads the settings from the InspectorPageAgent. This is necessary, to preserve the settings when changing the page with the WebInspector open.

pfeldman can you please have a look?

[Pavel Feldman]

Comment on attachment 177348 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=177348&action=review

This patch looks good (except for InspectorPageAgent interaction from within WebKit). There are two other ways it could be implemented: 1) finding a signal that would reliably trigger after layer tree is there from within WebCore or 2) creating such a signal.

> Source/WebKit/chromium/src/WebViewImpl.cpp:4048
> +#if ENABLE(INSPECTOR)

chromium === ENABLE(INSPECTOR)

> Source/WebKit/chromium/src/WebViewImpl.cpp:4049
> +            InspectorPageAgent* pageAgent = page()->inspectorController()->pageAgent();

You can only talk to InspectorController from WebKit layer.

Created attachment 177369 [details]
Patch

(In reply to comment #3)
> (From update of attachment 177348 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=177348&action=review
> 
> This patch looks good (except for InspectorPageAgent interaction from within WebKit). There are two other ways it could be implemented: 1) finding a signal that would reliably trigger after layer tree is there from within WebCore or 2) creating such a signal.
> 
> > Source/WebKit/chromium/src/WebViewImpl.cpp:4048
> > +#if ENABLE(INSPECTOR)
> 
> chromium === ENABLE(INSPECTOR)
> 
> > Source/WebKit/chromium/src/WebViewImpl.cpp:4049
> > +            InspectorPageAgent* pageAgent = page()->inspectorController()->pageAgent();
> 
> You can only talk to InspectorController from WebKit layer.

Thanks, I switched to storing the values in fields of WebViewImpl.

[WebKit Review Bot]

Comment on attachment 177369 [details]
Patch

Rejecting attachment 177369 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
ripts/update-webkit line 152.

Failed to run "['Tools/Scripts/update-webkit', '--chromium', '--force-update']" exit_code: 2
Updating OpenSource
From http://git.chromium.org/external/Webkit
   ef0ca77..8cbb25f  HEAD       -> origin/HEAD
error: Ref refs/remotes/origin/master is at 8cbb25f3febaaee8d69fbea1e5b98dd0173b71a9 but expected ef0ca77a554ed2a07b001ee4935266d6301b2bf8
 ! ef0ca77..8cbb25f  master     -> origin/master  (unable to update local ref)
Died at Tools/Scripts/update-webkit line 152.

Full output: http://queues.webkit.org/results/15120716

[WebKit Review Bot]

Comment on attachment 177369 [details]
Patch

Clearing flags on attachment: 177369

Committed r136529: <http://trac.webkit.org/changeset/136529>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.