Bug 39957 - Chromium: save inspector settings as dictionary, not as string.
Summary: Chromium: save inspector settings as dictionary, not as string.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-31 08:28 PDT by Pavel Podivilov
Modified: 2010-06-01 02:06 PDT (History)
1 user (show)

See Also:

Attachments
Proposed patch. (7.68 KB, patch)
2010-05-31 08:35 PDT, Pavel Podivilov 		pfeldman: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Podivilov 2010-05-31 08:28:52 PDT 
Chromium: save inspector settings as dictionary, not as string.
Comment 1 Pavel Podivilov 2010-05-31 08:35:10 PDT 
Created attachment 57472 [details]
Proposed patch.
Comment 2 Pavel Feldman 2010-05-31 09:14:49 PDT 
Comment on attachment 57472 [details]
Proposed patch.

r+ with nits.

WebKit/chromium/src/InspectorClientImpl.cpp:96
 +      m_inspectedWebView->inspectorSetting(key, &string);
So it did not compile for you with simply putting 'value' as a second argument?


WebKit/chromium/src/WebViewImpl.h:491
 +      typedef HashMap<WebCore::String, WebCore::String> SettingsMap;
WebKit should operate WebKit strings. I know it was wrong, but it may be a good time to fix it.
Comment 3 Pavel Podivilov 2010-05-31 10:00:09 PDT 
> WebKit/chromium/src/InspectorClientImpl.cpp:96
>  +      m_inspectedWebView->inspectorSetting(key, &string);
> So it did not compile for you with simply putting 'value' as a second argument?

Yep, it did not.

> 
> 
> WebKit/chromium/src/WebViewImpl.h:491
>  +      typedef HashMap<WebCore::String, WebCore::String> SettingsMap;
> WebKit should operate WebKit strings. I know it was wrong, but it may be a good time to fix it.

We need a map here, because InspectorClient will ask for inspector settings by key.
Comment 4 Pavel Feldman 2010-06-01 02:06:20 PDT 
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebKit/chromium/ChangeLog
	M	WebKit/chromium/public/WebView.h
	M	WebKit/chromium/public/WebViewClient.h
	M	WebKit/chromium/src/InspectorClientImpl.cpp
	M	WebKit/chromium/src/InspectorClientImpl.h
	M	WebKit/chromium/src/WebViewImpl.cpp
	M	WebKit/chromium/src/WebViewImpl.h
Committed r60469