Closed Bug 422792 Opened 12 years ago Closed 12 years ago

reduce narrow windows API calls in widget

Categories

(Core :: General, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: blassey, Assigned: crowderbt)

References

Details

Attachments

(1 file, 2 obsolete files)

No description provided.
Attachment #309238 - Flags: review?(roc)
Am I supposed to review this before 1.9 is done?
no rush.  As I understand it, it won't make it in anyway
     if (mLastSound) {
-      memcpy(mLastSound, data, dataLen);
-      data = mLastSound;
-      flags |= SND_ASYNC;
+      MultiByteToWideChar(CP_ACP,0, reinterpret_cast<const char*>(data), dataLen,mLastSound, 256);
+      flags |= SND_ASYNC;      
+      ::PlaySoundW(mLastSound, 0, flags);
+    }else{      
+#ifndef WINCE
+      ::PlaySoundA(reinterpret_cast<const char*>(data), 0, flags);
+#endif
     }

Why do we need to call PlaySoundA here?

+LPCTSTR nsWindow::WindowClass()
+const LPCTSTR kTClassNameHidden       = TEXT("MozillaHiddenWindowClass");
+const LPCTSTR kTClassNameUI           = TEXT("MozillaUIWindowClass");
+const LPCTSTR kTClassNameContent      = TEXT("MozillaContentWindowClass");
+const LPCTSTR kTClassNameContentFrame = TEXT("MozillaContentFrameWindowClass");
+const LPCTSTR kTClassNameGeneral      = TEXT("MozillaWindowClass");
+const LPCTSTR kTClassNameDialog       = TEXT("MozillaDialogClass");

Why aren't these all LPCWSTR and L""?
Attached patch v2 (obsolete) — Splinter Review
Assignee: nobody → crowder
Attachment #309238 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #309238 - Flags: review?(roc)
Comment on attachment 333299 [details] [diff] [review]
v2

I put the old sound API stuff back the way it was, generally, and took out the string constants that weren't being used anyway.
Attachment #333299 - Flags: review?(roc)
Comment on attachment 333299 [details] [diff] [review]
v2

+    format = ::RegisterClipboardFormatW(NS_ConvertASCIItoUTF16(aMimeStr).get());
+
+

Where'd those blank lines come from? Get rid of them.
Attachment #333299 - Flags: review?(roc) → review+
changeset:   16593:d1be05700e20

Thanks a lot for the snappy reviews!
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Had to back out because I landed on a closed tree.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
re: comment #9 -- thanks, I'll add those and do another spin for tomorrow.
(In reply to comment #9)
> still use ANSI API even if you apply sugest patch...
> 
> http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsWindow.cpp#1635
> http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsWindow.cpp#1716

I think changing these calls is unnecessary.  #ifdef UNICODE (aiui), these calls are replaced by GetWindowLongW(), anyway.
changeset:   16688:c7264525ad20
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
Had to back this out due to build problems it caused somewhere.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 333299 [details] [diff] [review]
v2

>@@ -308,23 +310,23 @@ nsresult nsClipboard::GetGlobalData(HGLO
>     FormatMessage(
>         FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM,
>         NULL,
>         GetLastError(),
>         MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), // Default language
>         (LPTSTR) &lpMsgBuf,
>         0,
>         NULL 
>     );
Doesn't this need to be FormatMessageW?

>@@ -113,23 +113,22 @@ NS_IMETHODIMP nsSound::OnStreamComplete(
>     return aStatus;
>   }
> 
>   PurgeLastSound();
> 
>   if (data && dataLen > 0) {
>     DWORD flags = SND_MEMORY | SND_NODEFAULT;
>     // We try to make a copy so we can play it async.
>-    mLastSound = (PRUint8 *) malloc(dataLen);
>+    mLastSound = (PRUint8 *) malloc(512);
>     if (mLastSound) {
>       memcpy(mLastSound, data, dataLen);
>       data = mLastSound;
>       flags |= SND_ASYNC;
>     }
This makes no sense.

>     ::PlaySound(reinterpret_cast<const char*>(data), 0, flags);
PlaySoundW?

>-LPCSTR nsWindow::WindowClass()
>+LPCTSTR nsWindow::WindowClass()
>-LPCSTR nsWindow::WindowPopupClass()
>+LPCTSTR nsWindow::WindowPopupClass()
These methods haven't been used for six years...
Attached patch v3Splinter Review
Updating to include some of Neil's suggestions.  I didn't remove the "unused" functions mentioned; I'll leave that for another bug, unless you insist on it (roc).
Attachment #333299 - Attachment is obsolete: true
Attachment #334762 - Flags: review?(roc)
Blocks: 452794
No longer blocks: 452794
Depends on: 452794
No longer depends on: 452794
http://hg.mozilla.org/mozilla-central/rev/f4bc794256c2

Landed this on 8/29, sorry for not closing (wasn't sure it'd stick).
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.