We need to make WebKit API for chromium support draggable region change update.
Created attachment 155681 [details] Proposed Patch
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Attachment 155681 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/Platform/ChangeLog', u'Source/Platf..." exit_code: 1 Source/WebKit/chromium/src/WebDocument.cpp:248: draggable_regions is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 155685 [details] Proposed Patch
Comment on attachment 155685 [details] Proposed Patch View in context: https://bugs.webkit.org/attachment.cgi?id=155685&action=review > Source/Platform/chromium/public/WebDraggableRegion.h:40 > +struct WebDraggableRegion { This looks like a wrapper for DashboardRegionValue, which is a WebCore concept, not a WebCore/platform concept. Therefore, this header should be in WebKit/chromium/public rather than in Platform. > Source/Platform/chromium/public/WebDraggableRegion.h:44 > + int type; Is this really an enum? Should we create an API for the enum values so we can COMPILE_ASSERT that they match their WebCore counterparts? > Source/WebKit/chromium/public/WebViewClient.h:384 > + // Draggable regions ---------------------------------------------------- Two blank lines before section headings.
(In reply to comment #5) > (From update of attachment 155685 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=155685&action=review > > > Source/Platform/chromium/public/WebDraggableRegion.h:40 > > +struct WebDraggableRegion { > > This looks like a wrapper for DashboardRegionValue, which is a WebCore concept, not a WebCore/platform concept. Therefore, this header should be in WebKit/chromium/public rather than in Platform. Will fix it at landing time. > > > Source/Platform/chromium/public/WebDraggableRegion.h:44 > > + int type; > > Is this really an enum? Should we create an API for the enum values so we can COMPILE_ASSERT that they match their WebCore counterparts? WebCore counterparts also use int type. For now, I am going to keep int type for our version. I am thinking of ignoring the type information (rectangular or circular). We will extract the shape information from the element when CSS shape is fully supported and then update this struct. > > > Source/WebKit/chromium/public/WebViewClient.h:384 > > + // Draggable regions ---------------------------------------------------- > > Two blank lines before section headings. Will fix it at landing time.
> WebCore counterparts also use int type. For now, I am going to keep int type for our version. I am thinking of ignoring the type information (rectangular or circular). We will extract the shape information from the element when CSS shape is fully supported and then update this struct. Just a bare int doesn't make any sense in the API. Perhaps we should omit it for now and add it later when we need it?
(In reply to comment #7) > > WebCore counterparts also use int type. For now, I am going to keep int type for our version. I am thinking of ignoring the type information (rectangular or circular). We will extract the shape information from the element when CSS shape is fully supported and then update this struct. > > Just a bare int doesn't make any sense in the API. Perhaps we should omit it for now and add it later when we need it? Sounds good.
Committed as http://trac.webkit.org/changeset/124394.