Bug 103937 - [chromium] fix showFPScounter and showPaintRects not toggleable after page change
Summary: [chromium] fix showFPScounter and showPaintRects not toggleable after page ch...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-03 15:17 PST by egraether
Modified: 2012-12-04 10:48 PST (History)
11 users (show)

See Also:


Attachments
Patch (5.59 KB, patch)
2012-12-03 15:21 PST, egraether
no flags Details | Formatted Diff | Diff
Patch (3.51 KB, patch)
2012-12-03 16:40 PST, egraether
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description egraether 2012-12-03 15:17:45 PST
[chromium] fix showFPScounter and showPaintRects not toggleable after page change
Comment 1 egraether 2012-12-03 15:21:48 PST
Created attachment 177348 [details]
Patch
Comment 2 egraether 2012-12-03 15:36:46 PST
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?
Comment 3 Pavel Feldman 2012-12-03 16:00:05 PST
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.
Comment 4 egraether 2012-12-03 16:40:08 PST
Created attachment 177369 [details]
Patch
Comment 5 egraether 2012-12-03 16:41:32 PST
(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.
Comment 6 WebKit Review Bot 2012-12-03 23:53:11 PST
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
Comment 7 WebKit Review Bot 2012-12-04 10:48:47 PST
Comment on attachment 177369 [details]
Patch

Clearing flags on attachment: 177369

Committed r136529: <http://trac.webkit.org/changeset/136529>
Comment 8 WebKit Review Bot 2012-12-04 10:48:50 PST
All reviewed patches have been landed.  Closing bug.