Implement a basic PPAPI plugin for Blink layout tests.

Implement a basic PPAPI plugin for Blink layout tests.

The plugin is automatically registered in layout tests using the
MIME type application/x-blink-test-plugin. Several layout tests
have also been migrated to use the new plugin.

Implementing the plugin also revealed a bug in PPAPI and the
layout test runner:
- Retrieving the key code via PPAPI always returned KeyA on Mac.
- The layout test runner explicitly set DYLD_FRAMEWORK_PATH,
  making it hard to load the plugin from the right location.

BUG=474535
Committed: https://crrev.com/0d93c1a79b3c4cc3b633e9f6e1fbae6cf1611f64
Cr-Commit-Position: refs/heads/master@{#362872}

[dcheng, 2015-11-30 10:47:59]

dcheng@chromium.org changed reviewers:
+ avi@chromium.org, piman@chromium.org

[dcheng, 2015-11-30 10:49:00]

dcheng@chromium.org changed reviewers:
+ tkent@chromium.org

[dcheng, 2015-11-30 10:49:38]

dcheng@chromium.org changed reviewers:
+ esprehn@chromium.org
- tkent@chromium.org

[dcheng, 2015-12-01 01:03:30]

+avi for content, +piman for ppapi, +esprehn for third_party/WebKit

https://codereview.chromium.org/1480303002/diff/60001/content/content_shell.gypi
File content/content_shell.gypi (right):

https://codereview.chromium.org/1480303002/diff/60001/content/content_shell.g...
content/content_shell.gypi:739: 'destination':
'<(PRODUCT_DIR)/$(CONTENTS_FOLDER_PATH)',
This is slightly horrible, but it seems to be the easiest way to integrate with
the assumption that PPAPI test plugins live in DIR_MODULE.

Otherwise, a Mac static_library content_shell build fails to find the test
plugin.

https://codereview.chromium.org/1480303002/diff/60001/ppapi/tests/blink_test_...
File ppapi/tests/blink_test_plugin.cc (right):

https://codereview.chromium.org/1480303002/diff/60001/ppapi/tests/blink_test_...
ppapi/tests/blink_test_plugin.cc:116: // Template magic to cover the lack of
base::StringPrintf.
Apparently parts of //base might be allowed in ppapi now. That seems like a yak
shave for another time though...

https://codereview.chromium.org/1480303002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/replaced/invalid-object-with-fallback.html
(right):

https://codereview.chromium.org/1480303002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/replaced/invalid-object-with-fallback.html:21:
document.querySelector("embed").addEventListener("message", function (message) {
Is there a better way to tell if a PPAPI plugin loaded successfully or not?

There's a helper function (see below) that somehow guarantees that the closure
you pass it will be executed after plugins load, but I don't understand how it
works.

Without any way to detect if a plugin is actually loaded or not though, I can't
use it here.

https://codereview.chromium.org/1480303002/diff/60001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/plugins/mouse-events-fixedpos.html (right):

https://codereview.chromium.org/1480303002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/plugins/mouse-events-fixedpos.html:23:
startAfterLoadAndFinish(function () {
I discovered this function at the end of updating this set of tests.

I have no idea how it works, but I'm going to update keyboard-events.html and
mouse-events.html to use it, since it seems simpler.

[piman, 2015-12-01 01:13:27]

lgtm

[Avi (use Gerrit), 2015-12-01 15:01:14]

https://codereview.chromium.org/1480303002/diff/100001/content/public/test/pp...
File content/public/test/ppapi_test_utils.cc (right):

https://codereview.chromium.org/1480303002/diff/100001/content/public/test/pp...
content/public/test/ppapi_test_utils.cc:25: printf("Couldn't look up module
path!\n");
Why doesn't LOG(ERROR) or LOG(FATAL) work here?

https://codereview.chromium.org/1480303002/diff/100001/content/public/test/pp...
content/public/test/ppapi_test_utils.cc:31: printf("Looking for plugin in %s\n",
plugin_path.value().c_str());
Is this needed?

https://codereview.chromium.org/1480303002/diff/100001/content/public/test/pp...
content/public/test/ppapi_test_utils.cc:34: printf("File doesn't exist!\n");
LOG(FATAL)?

[dcheng, 2015-12-01 19:52:02]

https://codereview.chromium.org/1480303002/diff/100001/content/public/test/pp...
File content/public/test/ppapi_test_utils.cc (right):

https://codereview.chromium.org/1480303002/diff/100001/content/public/test/pp...
content/public/test/ppapi_test_utils.cc:25: printf("Couldn't look up module
path!\n");
On 2015/12/01 at 15:01:14, Avi wrote:
> Why doesn't LOG(ERROR) or LOG(FATAL) work here?

It does, but these are temporary debugging lines. I'm currently trying to figure
out why:

content_shell --run-layout-test plugins/keyboard-events.html

works but:

blink/tools/run_layout_tests.sh --debug plugins/keyboard-events.html

does not.

Btw, this is a Mac specific issue, so if you have any ideas ;-)

[dcheng, 2015-12-01 20:22:42]

The CQ bit was checked by dcheng@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-01 20:26:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1480303002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1480303002/120001

[Avi (use Gerrit), 2015-12-01 20:51:55]

https://codereview.chromium.org/1480303002/diff/100001/content/public/test/pp...
File content/public/test/ppapi_test_utils.cc (right):

https://codereview.chromium.org/1480303002/diff/100001/content/public/test/pp...
content/public/test/ppapi_test_utils.cc:25: printf("Couldn't look up module
path!\n");
On 2015/12/01 19:52:02, dcheng wrote:
> On 2015/12/01 at 15:01:14, Avi wrote:
> > Why doesn't LOG(ERROR) or LOG(FATAL) work here?
> 
> It does, but these are temporary debugging lines. I'm currently trying to
figure
> out why:
> 
> content_shell --run-layout-test plugins/keyboard-events.html
> 
> works but:
> 
> blink/tools/run_layout_tests.sh --debug plugins/keyboard-events.html
> 
> does not.
> 
> Btw, this is a Mac specific issue, so if you have any ideas ;-)

No ideas offhand, but don't commit debugging code.

[dcheng, 2015-12-01 22:08:18]

dcheng@chromium.org changed reviewers:
+ dpranke@chromium.org

[dcheng, 2015-12-01 22:08:18]

PTAL.

I guess it wasn't clear, but PS 5 and PS 6 were purely debugging patchsets (and
not meant to be committed) to try to figure out what was failing on the mac
trybot. I later managed to get it to fail locally as well, so all the debugging
stuff has been reverted.

+dpranke as well for the webkitpy changes.

https://codereview.chromium.org/1480303002/diff/120001/third_party/WebKit/Too...
File third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base.py (left):

https://codereview.chromium.org/1480303002/diff/120001/third_party/WebKit/Too...
third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base.py:1106:
clean_env['DYLD_FRAMEWORK_PATH'] = self._build_path()
This was changed in https://codereview.chromium.org/22978015, but doesn't appear
to actually be necessary (I tried a component build as well, and it worked
fine).

[commit-bot: I haz the power, 2015-12-01 22:27:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-01 22:27:21]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Dirk Pranke, 2015-12-01 22:45:22]

webkitpy lgtm

[raymes, 2015-12-01 23:43:41]

raymes@chromium.org changed reviewers:
+ raymes@chromium.org

[raymes, 2015-12-01 23:43:42]

ppapi/ lgtm

https://codereview.chromium.org/1480303002/diff/120001/ppapi/tests/blink_test...
File ppapi/tests/blink_test_plugin.cc (right):

https://codereview.chromium.org/1480303002/diff/120001/ppapi/tests/blink_test...
ppapi/tests/blink_test_plugin.cc:72: void HandleMessage(const pp::Var&
message_data) override {}
nit: can you remove this line?

[dcheng, 2015-12-01 23:51:17]

https://codereview.chromium.org/1480303002/diff/120001/ppapi/tests/blink_test...
File ppapi/tests/blink_test_plugin.cc (right):

https://codereview.chromium.org/1480303002/diff/120001/ppapi/tests/blink_test...
ppapi/tests/blink_test_plugin.cc:72: void HandleMessage(const pp::Var&
message_data) override {}
On 2015/12/01 at 23:43:42, raymes wrote:
> nit: can you remove this line?

Done.

https://codereview.chromium.org/1480303002/diff/140001/content/renderer/peppe...
File content/renderer/pepper/event_conversion.cc (left):

https://codereview.chromium.org/1480303002/diff/140001/content/renderer/peppe...
content/renderer/pepper/event_conversion.cc:139: // Workaround for |domCode| not
being set on OS X. crbug.com/493833
I think this should fix the Mac test... hopefully.

[piman, 2015-12-01 23:58:51]

lgtm

[Avi (use Gerrit), 2015-12-02 00:24:43]

lgtm

Oh, OK. Yeah, that's fine.

[esprehn, 2015-12-02 03:46:53]

lgtm, you're a hero!

https://codereview.chromium.org/1480303002/diff/160001/ppapi/tests/blink_test...
File ppapi/tests/blink_test_plugin.cc (right):

https://codereview.chromium.org/1480303002/diff/160001/ppapi/tests/blink_test...
ppapi/tests/blink_test_plugin.cc:15: static void DummyCompletionCallback(void*,
int32_t) {}
should this code be in some kind of namespace? I don't know the rules for stuff
in ppapi/tests

[dcheng, 2015-12-02 22:32:38]

Description was changed from

==========
Implement a basic PPAPI plugin for Blink layout tests.

BUG=474535
==========

to

==========
Implement a basic PPAPI plugin for Blink layout tests.

The plugin is automatically registered in layout tests using the
MIME type application/x-blink-test-plugin. Several layout tests
have also been migrated to use the new plugin.

Implementing the plugin also revealed a bug in PPAPI and the
layout test runner:
- Retrieving the key code via PPAPI always returned KeyA on Mac.
- The layout test runner explicitly set DYLD_FRAMEWORK_PATH,
  making it hard to load the plugin from the right location.

BUG=474535
==========

[dcheng, 2015-12-02 22:35:31]

The CQ bit was checked by dcheng@chromium.org

[dcheng, 2015-12-02 22:35:32]

I also added more comments, because yay, comments.

https://codereview.chromium.org/1480303002/diff/160001/ppapi/tests/blink_test...
File ppapi/tests/blink_test_plugin.cc (right):

https://codereview.chromium.org/1480303002/diff/160001/ppapi/tests/blink_test...
ppapi/tests/blink_test_plugin.cc:15: static void DummyCompletionCallback(void*,
int32_t) {}
On 2015/12/02 at 03:46:53, esprehn wrote:
> should this code be in some kind of namespace? I don't know the rules for
stuff in ppapi/tests

Done. It doesn't matter here, but it's probably better practice to anyway.

[dcheng, 2015-12-02 22:35:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpranke@chromium.org, raymes@chromium.org, avi@chromium.org, piman@chromium.org,
esprehn@chromium.org
Link to the patchset: https://codereview.chromium.org/1480303002/#ps240001
(title: "MOAR COMMENTS")

[commit-bot: I haz the power, 2015-12-02 22:37:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1480303002/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1480303002/240001

[commit-bot: I haz the power, 2015-12-02 23:17:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-02 23:17:29]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dcheng, 2015-12-02 23:18:48]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2015-12-02 23:19:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1480303002/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1480303002/240001

[commit-bot: I haz the power, 2015-12-02 23:38:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-02 23:38:23]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dcheng, 2015-12-02 23:42:59]

The CQ bit was checked by dcheng@chromium.org

[dcheng, 2015-12-02 23:42:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpranke@chromium.org, avi@chromium.org, esprehn@chromium.org,
raymes@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1480303002/#ps260001
(title: "Fix Android after https://codereview.chromium.org/1478633002")

[commit-bot: I haz the power, 2015-12-02 23:45:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1480303002/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1480303002/260001

[commit-bot: I haz the power, 2015-12-03 02:46:51]

Committed patchset #14 (id:260001)

[commit-bot: I haz the power, 2015-12-03 02:47:31]

Description was changed from

==========
Implement a basic PPAPI plugin for Blink layout tests.

The plugin is automatically registered in layout tests using the
MIME type application/x-blink-test-plugin. Several layout tests
have also been migrated to use the new plugin.

Implementing the plugin also revealed a bug in PPAPI and the
layout test runner:
- Retrieving the key code via PPAPI always returned KeyA on Mac.
- The layout test runner explicitly set DYLD_FRAMEWORK_PATH,
  making it hard to load the plugin from the right location.

BUG=474535
==========

to

==========
Implement a basic PPAPI plugin for Blink layout tests.

The plugin is automatically registered in layout tests using the
MIME type application/x-blink-test-plugin. Several layout tests
have also been migrated to use the new plugin.

Implementing the plugin also revealed a bug in PPAPI and the
layout test runner:
- Retrieving the key code via PPAPI always returned KeyA on Mac.
- The layout test runner explicitly set DYLD_FRAMEWORK_PATH,
  making it hard to load the plugin from the right location.

BUG=474535

Committed: https://crrev.com/0d93c1a79b3c4cc3b633e9f6e1fbae6cf1611f64
Cr-Commit-Position: refs/heads/master@{#362872}
==========

[commit-bot: I haz the power, 2015-12-03 02:47:32]

Patchset 14 (id:??) landed as
https://crrev.com/0d93c1a79b3c4cc3b633e9f6e1fbae6cf1611f64
Cr-Commit-Position: refs/heads/master@{#362872}