Bug 57327 - Web Inspector: extract InspectorBrowserAgent from InspectorAgent
Summary: Web Inspector: extract InspectorBrowserAgent from InspectorAgent
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Ilya Tikhonovsky
URL:
Keywords:
Depends on: 57364
Blocks:
  Show dependency treegraph
 
Reported: 2011-03-29 04:21 PDT by Ilya Tikhonovsky
Modified: 2011-04-01 23:04 PDT (History)
13 users (show)

See Also:


Attachments
[patch] first version (63.02 KB, patch)
2011-03-29 04:25 PDT, Ilya Tikhonovsky
no flags Details | Formatted Diff | Diff
[patch] rebaselined. style fixed (62.99 KB, patch)
2011-03-29 04:38 PDT, Ilya Tikhonovsky
pfeldman: review+
Details | Formatted Diff | Diff
Inspector.json (64.53 KB, text/plain)
2011-03-29 07:22 PDT, Ilya Tikhonovsky
no flags Details
Inspector.json (64.65 KB, text/plain)
2011-03-29 07:25 PDT, Ilya Tikhonovsky
no flags Details
the patch I'm going to land (62.92 KB, patch)
2011-03-29 11:03 PDT, Ilya Tikhonovsky
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ilya Tikhonovsky 2011-03-29 04:21:36 PDT
There are browser related methods and inspector related methods in InspectorAgent.  
It would be nice to extract browser specific methods for future usage the rest of methods in workers debugger.
Comment 1 Ilya Tikhonovsky 2011-03-29 04:25:50 PDT
Created attachment 87297 [details]
[patch] first version
Comment 2 WebKit Review Bot 2011-03-29 04:28:24 PDT
Attachment 87297 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/http..." exit_code: 1

Source/WebCore/inspector/InspectorBrowserAgent.h:70:  The parameter name "frame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/inspector/InspectorBrowserAgent.h:70:  The parameter name "world" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/inspector/InspectorBrowserAgent.h:74:  The parameter name "frontend" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/inspector/InspectorBrowserAgent.cpp:38:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 4 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Ilya Tikhonovsky 2011-03-29 04:38:16 PDT
Created attachment 87298 [details]
[patch] rebaselined. style fixed
Comment 4 Ilya Tikhonovsky 2011-03-29 07:22:33 PDT
Created attachment 87314 [details]
Inspector.json
Comment 5 Ilya Tikhonovsky 2011-03-29 07:25:11 PDT
Created attachment 87315 [details]
Inspector.json
Comment 6 Pavel Feldman 2011-03-29 07:40:15 PDT
Comment on attachment 87298 [details]
[patch] rebaselined. style fixed

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

> Source/WebCore/inspector/InspectorAgent.cpp:173
> +    m_browserAgent->restore(inspectedURL().string());

PageAgent should simply get url from the page.
Comment 7 Ilya Tikhonovsky 2011-03-29 11:02:08 PDT
landed r82281
Comment 8 Ilya Tikhonovsky 2011-03-29 11:03:52 PDT
Created attachment 87368 [details]
the patch I'm going to land
Comment 9 WebKit Review Bot 2011-03-29 11:07:00 PDT
http://trac.webkit.org/changeset/82281 might have broken Qt Linux Release minimal and Qt Windows 32-bit Release
Comment 10 Yury Semikhatsky 2011-04-01 00:37:46 PDT
Comment on attachment 87368 [details]
the patch I'm going to land

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

> Source/WebCore/inspector/InspectorPageAgent.cpp:35
> +#if ENABLE(INSPECTOR) && ENABLE(JAVASCRIPT_DEBUGGER)

The page agent should be enabled even if JAVASCRIPT_DEBUGGER is off.

> Source/WebCore/inspector/InspectorPageAgent.h:34
> +#if ENABLE(JAVASCRIPT_DEBUGGER) && ENABLE(INSPECTOR)

ENABLE(JAVASCRIPT_DEBUGGER) guard should be removed.
Comment 11 Ilya Tikhonovsky 2011-04-01 23:04:50 PDT
(In reply to comment #10)
> (From update of attachment 87368 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=87368&action=review
> 
> > Source/WebCore/inspector/InspectorPageAgent.cpp:35
> > +#if ENABLE(INSPECTOR) && ENABLE(JAVASCRIPT_DEBUGGER)
> 
> The page agent should be enabled even if JAVASCRIPT_DEBUGGER is off.
> 
> > Source/WebCore/inspector/InspectorPageAgent.h:34
> > +#if ENABLE(JAVASCRIPT_DEBUGGER) && ENABLE(INSPECTOR)
> 
> ENABLE(JAVASCRIPT_DEBUGGER) guard should be removed.

followup patch landed as r82767