Bug 95254 – Remove unused methods from Chromium WebKit API

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Nasko Oskov]

As part of https://bugs.webkit.org/show_bug.cgi?id=93127, I have introduced two new methods in the Chromium WebKit API. I'm removing the usage of the old one from the Chrome side and the old versions need to be removed from WebKit as well.

[Nasko Oskov]

Created attachment 161075 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Adam Barth]

Comment on attachment 161075 [details]
Patch

I assume you checked that all the implementations of willCheckAndDispatchMessageEvent now accept the extra argument.

[Nasko Oskov]

(In reply to comment #3)
> (From update of attachment 161075 [details])
> I assume you checked that all the implementations of willCheckAndDispatchMessageEvent now accept the extra argument.

The only one I found was the RenderVIewImpl one, which I've already migrated to the new version. In addition, a full build of Chromium completed without any errors.

[WebKit Review Bot]

Comment on attachment 161075 [details]
Patch

Attachment 161075 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13660483

[Adam Barth]

You might need to update http://trac.webkit.org/browser/trunk/Source/WebKit/chromium/DEPS

[Peter Beverloo (cr-android ews)]

Comment on attachment 161075 [details]
Patch

Attachment 161075 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/13653529

[Nasko Oskov]

(In reply to comment #7)
> (From update of attachment 161075 [details])
> Attachment 161075 [details] did not pass cr-android-ews (chromium-android):
> Output: http://queues.webkit.org/results/13653529

This is expected for the time being. This change depends on a Chromium change currently in review: https://chromiumcodereview.appspot.com/10879105/. Once it is committed, this patch will be sent to the CQ and should succeed.

[Nasko Oskov]

Created attachment 161575 [details]
Patch

[Nasko Oskov]

Created attachment 161583 [details]
Patch

[WebKit Review Bot]

Comment on attachment 161583 [details]
Patch

Attachment 161583 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13695902

[Peter Beverloo (cr-android ews)]

Comment on attachment 161583 [details]
Patch

Attachment 161583 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/13680948

[Adam Barth]

Comment on attachment 161583 [details]
Patch

Did the DEPS roll land?  Let's find out.

[WebKit Review Bot]

Comment on attachment 161583 [details]
Patch

Rejecting attachment 161583 [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:
target/glue/Source/WebKit/chromium/webkit/glue/webpreferences.o
  CXX(target) out/Release/obj.target/glue/Source/WebKit/chromium/webkit/glue/websocketstreamhandle_impl.o
cc1plus: warnings being treated as errors
At global scope:
cc1plus: error: unrecognized command line option "-Wno-narrowing"
cc1plus: error: unrecognized command line option "-Wno-narrowing"
make: *** [out/Release/obj.target/glue/Source/WebKit/chromium/webkit/glue/webkit_glue.o] Error 1
make: *** Waiting for unfinished jobs....

Full output: http://queues.webkit.org/results/13725189

[Nasko Oskov]

No, the DEPS didn't roll out. The bug for rolling is https://bugs.webkit.org/show_bug.cgi?id=95511.

[Nasko Oskov]

And the DEPS are now rolled with http://trac.webkit.org/changeset/127305. Can you cq+ again?

[WebKit Review Bot]

Comment on attachment 161583 [details]
Patch

Rejecting attachment 161583 [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:
WebFrame' has no member named 'name'
Tools/DumpRenderTree/chromium/TestShell.cpp: In function 'void dumpFrameScrollPosition(WebKit::WebFrame*, bool)':
Tools/DumpRenderTree/chromium/TestShell.cpp:450: error: 'class WebKit::WebFrame' has no member named 'name'
  CXX(target) out/Release/obj.target/DumpRenderTree/Tools/DumpRenderTree/chromium/TestShellPosix.o
make: *** [out/Release/obj.target/DumpRenderTree/Tools/DumpRenderTree/chromium/TestShell.o] Error 1
make: *** Waiting for unfinished jobs....

Full output: http://queues.webkit.org/results/13714391

[Nasko Oskov]

Created attachment 164242 [details]
Patch

[WebKit Review Bot]

Comment on attachment 164242 [details]
Patch

Attachment 164242 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13851692

New failing tests:
platform/chromium/fast/events/intercept-postmessage.html

[Adam Barth]

That looks like it could be a real bug...  Perhaps DumpRenderTree is still using the API?

[WebKit Review Bot]

Comment on attachment 164242 [details]
Patch

Attachment 164242 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13851709

New failing tests:
platform/chromium/fast/events/intercept-postmessage.html

[Nasko Oskov]

Created attachment 166091 [details]
Patch

[Nasko Oskov]

I had missed updating willCheckAndDispatchMessageEvent in WebViewHost for DumpRenderTree. It compiled fine, but didn't resolve to the right method call. Updated and the test passes.

[WebKit Review Bot]

Comment on attachment 166091 [details]
Patch

Clearing flags on attachment: 166091

Committed r129821: <http://trac.webkit.org/changeset/129821>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.