Bug 138343 - Web Inspector: Enum value collisions between different generators
Summary: Web Inspector: Enum value collisions between different generators
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-11-03 18:40 PST by Joseph Pecoraro
Modified: 2014-11-04 07:36 PST (History)
6 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (53.25 KB, patch)
2014-11-03 18:54 PST, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2014-11-03 18:40:49 PST
* SUMMARY
Sending Debugger.pause reason "InspectorDebuggerFrontendDispatcher::Reason::Exception" across to the frontend actually sends the string "WebSocket" instead of "exception". There is a collision between enum values for the different dispatchers.

Generated code had both:

> // InspectorProtocolObjects.h
> 
>     namespace Timeline {
>     enum class EventType {
>         ...
>         WebSocketDestroy = 133,
>     };
>     }
> 
> // InspectorFrontendDispatchers.h
> 
>     class InspectorDebuggerFrontendDispatcher {
>     public:
>         ...
>         enum class Reason {
>             Exception = 133,
>         };
>     }

We need to ensure that the values we generate are unique across all enums in all domains, not just the domains being generated for that particular dispatcher.
Comment 1 Joseph Pecoraro 2014-11-03 18:40:58 PST
I have a fix, I need to make a test.
Comment 2 Radar WebKit Bug Importer 2014-11-03 18:41:11 PST
<rdar://problem/18862317>
Comment 3 Joseph Pecoraro 2014-11-03 18:54:29 PST
Created attachment 240899 [details]
[PATCH] Proposed Fix

Before the patch you can see the collision.

> FAIL: enum-values.json-result
> --- JavaScriptCore/inspector/scripts/tests/expected/enum-values.json-result	2014-11-03 18:50:12.000000000 -0800
> +++ /var/folders/_b/scj8wvc516g_708gsbsfwq5w0000gn/T/tmpg_18v0/enum-values.json-result	2014-11-03 18:50:33.000000000 -0800
> @@ -159,9 +159,9 @@
>      // Named after parameter 'returnValue' while generating command/event commandWithEnumReturnValue.
>      enum class ReturnValue {
>          Shared = 0,
> -        Cyan = 1,
> -        Magenta = 2,
> -        Yellow = 3,
> +        Cyan = 6,
> +        Magenta = 7,
> +        Yellow = 8,
>      }; // enum class ReturnValue
>      virtual void commandWithEnumReturnValue(ErrorString&, InspectorCommandDomainBackendDispatcherHandler::ReturnValue* out_returnValue) = 0;
>  protected:
> @@ -343,8 +343,8 @@
>          // Named after parameter 'parameter' while generating command/event eventWithEnumParameter.
>          enum class Parameter {
>              Shared = 0,
> -            Black = 1,
> -            White = 2,
> +            Black = 4,
> +            White = 5,
>          }; // enum class Parameter
>      void eventWithEnumParameter(Parameter parameter);
>  private:
Comment 4 WebKit Commit Bot 2014-11-03 18:57:18 PST
This patch modifies the inspector protocol generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-inspector-generator-tests --reset-results`)
Comment 5 Brian Burg 2014-11-04 06:56:54 PST
Comment on attachment 240899 [details]
[PATCH] Proposed Fix

Hmm, hadn't thought about that case.. thanks for the patch!
Comment 6 WebKit Commit Bot 2014-11-04 07:36:08 PST
Comment on attachment 240899 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 240899

Committed r175546: <http://trac.webkit.org/changeset/175546>
Comment 7 WebKit Commit Bot 2014-11-04 07:36:11 PST
All reviewed patches have been landed.  Closing bug.