Bug 48797

Summary: Respond to NP_GetProperty by sending a GetProperty message
Product: WebKit Reporter: Anders Carlsson <andersca>
Component: New BugsAssignee: Anders Carlsson <andersca>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch sullivan: review+

Description Anders Carlsson 2010-11-01 16:16:27 PDT
Respond to NP_GetProperty by sending a GetProperty message
Comment 1 Anders Carlsson 2010-11-01 16:30:25 PDT
Created attachment 72593 [details]
Patch
Comment 2 WebKit Review Bot 2010-11-01 16:32:27 PDT
Attachment 72593 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/Shared/Plugins/NPVariantData.h:32:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
WebKit2/Shared/Plugins/NPIdentifierData.h:35:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
WebKit2/Shared/Plugins/NPObjectProxy.cpp:144:  NPObjectProxy::NP_GetProperty is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 3 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 John Sullivan 2010-11-01 16:52:53 PDT
Comment on attachment 72593 [details]
Patch

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

> WebKit2/Shared/Plugins/NPObjectProxy.cpp:40
> +NPObjectProxy* NPObjectProxy::create(NPRemoteObjectMap* npRemoteObjectMap, uint64_t npObjectID)

Is it legal to pass 0 for the npRemoteObjectMap?

> WebKit2/Shared/Plugins/NPObjectProxy.cpp:80
> +        return false;

Should this nil-check m_npRemoteObjectMap?

> WebKit2/WebKit2.xcodeproj/project.pbxproj:2613
>  		};

Do you need to update the Visual Studio project file also?
Comment 4 Anders Carlsson 2010-11-01 16:55:18 PDT
(In reply to comment #3)
> (From update of attachment 72593 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=72593&action=review
> 
> > WebKit2/Shared/Plugins/NPObjectProxy.cpp:40
> > +NPObjectProxy* NPObjectProxy::create(NPRemoteObjectMap* npRemoteObjectMap, uint64_t npObjectID)
> 
> Is it legal to pass 0 for the npRemoteObjectMap?

No, I'll add an assert, I'll assert that npObjectID isn't null too while I'm at it.

> 
> > WebKit2/Shared/Plugins/NPObjectProxy.cpp:80
> > +        return false;
> 
> Should this nil-check m_npRemoteObjectMap?
> 

Yes.

> > WebKit2/WebKit2.xcodeproj/project.pbxproj:2613
> >  		};
> 
> Do you need to update the Visual Studio project file also?

Nope, this code is Mac specific for now.
Comment 5 Anders Carlsson 2010-11-01 16:59:03 PDT
Committed r71075: <http://trac.webkit.org/changeset/71075>
Comment 6 WebKit Review Bot 2010-11-01 18:00:01 PDT
http://trac.webkit.org/changeset/71076 might have broken GTK Linux 64-bit Debug
The following tests are not passing:
http/tests/websocket/tests/alert-in-event-handler.html
http/tests/websocket/tests/bad-handshake-crash.html
http/tests/websocket/tests/bufferedAmount-after-close.html
http/tests/websocket/tests/close-on-navigate-new-location.html
http/tests/websocket/tests/close-on-unload-and-force-gc.html
http/tests/websocket/tests/close-on-unload-reference-in-parent.html
http/tests/websocket/tests/close-on-unload.html
http/tests/websocket/tests/cross-origin.html
http/tests/websocket/tests/error-detect.html
http/tests/websocket/tests/frame-length-longer-than-buffer.html
http/tests/websocket/tests/frame-length-overflow.html
http/tests/websocket/tests/frame-length-skip.html
http/tests/websocket/tests/frame-lengths.html
http/tests/websocket/tests/handshake-challenge-randomness.html
http/tests/websocket/tests/handshake-error.html
http/tests/websocket/tests/handshake-fail-by-cross-origin.html
http/tests/websocket/tests/handshake-fail-by-no-cr.html
http/tests/websocket/tests/handshake-fail-by-sub-protocol-mismatch.html
http/tests/websocket/tests/httponly-cookie.pl
http/tests/websocket/tests/long-invalid-header.html