Hello andreip,
I'd like you to do a code review. Please execute
g4 diff -c 9736781
or point your web browser to
http://mondrian/9736781
to review the following code:
Change 9736781 by stevebl...@steveblock-gears1 on 2009/01/15 16:17:10 *pending*
Cleans up some resource linking ...
- On WinCE, add setup.res to the module DLL - this was inadvertently
removed when refactoring the autoupdater in CL 8749874).
- We don't need the custom buttons or alert dialog as resources on
WinCE as they're not used.
- We don't need buttons.css as a resource, as it's included inline in
the dialog HTML.
- Add ui_resources.res and module.res to the IE Mobile module DLL -
these were inadvertently removed when renaming IE to IEMOBILE on WinCE in CL
9700664.
R=andreip
[email protected]
DELTA=31 (27 added, 2 deleted, 2 changed)
OCL=9736781
Affected files ...
... //depot/googleclient/gears/opensource/gears/Makefile#228 edit
... //depot/googleclient/gears/opensource/gears/tools/rules.mk#100 edit
... //depot/googleclient/gears/opensource/gears/ui/ie/ui_resources.rc.m4#8 edit
31 delta lines: 27 added, 2 deleted, 2 changed
If you can't do the review, please let me know as soon as possible. During
your review, please ensure that all new code has corresponding unit tests and
that existing unit tests are updated appropriately. Visit
http://www/eng/code_review.html for more information.
This is a semiautomated message from "g4 mail". Complaints or suggestions?
Mail [email protected].
Change 9736781 by stevebl...@steveblock-gears1 on 2009/01/15 16:17:10 *pending*
Cleans up some resource linking ...
- On WinCE, add setup.res to the module DLL - this was inadvertently
removed when refactoring the autoupdater in CL 8749874).
- We don't need the custom buttons or alert dialog as resources on
WinCE as they're not used.
- We don't need buttons.css as a resource, as it's included inline in
the dialog HTML.
- Add ui_resources.res and module.res to the IE Mobile module DLL -
these were inadvertently removed when renaming IE to IEMOBILE on WinCE in CL
9700664.
Affected files ...
... //depot/googleclient/gears/opensource/gears/Makefile#228 edit
... //depot/googleclient/gears/opensource/gears/tools/rules.mk#100 edit
... //depot/googleclient/gears/opensource/gears/ui/ie/ui_resources.rc.m4#8 edit
==== //depot/googleclient/gears/opensource/gears/Makefile#228 -
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/Makefile ====
# action=edit type=text
--- googleclient/gears/opensource/gears/Makefile 2009-01-16
11:46:14.000000000 +0000
+++ googleclient/gears/opensource/gears/Makefile 2009-01-16
10:35:58.000000000 +0000
@@ -872,6 +872,10 @@
$(IE_OUTDIR)/module.res \
$(NULL)
+IEMOBILE_LINK_EXTRAS += \
+ $(IEMOBILE_OUTDIR)/module.res \
+ $(NULL)
+
#-----------------------------------------------------------------------------
# base/npapi
@@ -1558,6 +1562,10 @@
$(IE_OUTDIR)/ui_resources.res \
$(NULL)
+IEMOBILE_LINK_EXTRAS += \
+ $(IEMOBILE_OUTDIR)/ui_resources.res \
+ $(NULL)
+
ifeq ($(OS),wince)
# The string table uses multiple languages, which are not supported by
# LoadString on WinCE.
@@ -2059,7 +2067,11 @@
setup.cc \
$(NULL)
+# setup.res contains strings used by both the module DLL and setup.dll.
IEMOBILE_WINCESETUP_LINK_EXTRAS += \
+ $(IEMOBILE_OUTDIR)/setup.res \
+ $(NULL)
+IEMOBILE_LINK_EXTRAS += \
$(IEMOBILE_OUTDIR)/setup.res \
$(NULL)
@@ -2082,7 +2094,11 @@
setup.cc \
$(NULL)
+# setup.res contains strings used by both the module DLL and setup.dll.
OPERA_WINCESETUP_LINK_EXTRAS += \
+ $(OPERA_OUTDIR)/setup.res \
+ $(NULL)
+OPERA_LINK_EXTRAS += \
$(OPERA_OUTDIR)/setup.res \
$(NULL)
@@ -2212,6 +2228,7 @@
OPERA_IDLSRCS += $(NPAPI_IDLSRCS)
OPERA_HTML_M4SRCS += $(NPAPI_HTML_M4SRCS)
OPERA_LIBS += $(NPAPI_LIBS)
+# OPERA_LINK_EXTRAS is set independently of NPAPI_LINK_EXTRAS
endif
endif
@@ -2227,6 +2244,7 @@
IEMOBILE_IDLSRCS += $(IE_IDLSRCS)
IEMOBILE_HTML_M4SRCS += $(IE_HTML_M4SRCS)
IEMOBILE_LIBS += $(IE_LIBS)
+# IEMOBILE_LINK_EXTRAS is set independently of IE_LINK_EXTRAS
endif
endif
==== //depot/googleclient/gears/opensource/gears/tools/rules.mk#100 -
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/tools/rules.mk ====
# action=edit type=text
--- googleclient/gears/opensource/gears/tools/rules.mk 2009-01-16
11:46:14.000000000 +0000
+++ googleclient/gears/opensource/gears/tools/rules.mk 2009-01-16
00:09:41.000000000 +0000
@@ -1008,6 +1008,8 @@
$(ECHO) $(THIRD_PARTY_OBJS) | $(TRANSLATE_LINKER_FILE_LIST) >>
$(OUTDIR)/obj_list.temp
$(MKDLL) $(DLLFLAGS) $($(BROWSER)_DLLFLAGS) $($(BROWSER)_LINK_EXTRAS)
$($(BROWSER)_LIBS) $(SKIA_LIB) $(EXT_LINKER_CMD_FLAG)$(OUTDIR)/obj_list.temp
rm $(OUTDIR)/obj_list.temp
+endif # !symbian
+endif # !android
OPERA_OBJS1 = $(wordlist 1, 100, $(OPERA_OBJS))
OPERA_OBJS2 = $(wordlist 101, 999, $(OPERA_OBJS))
@@ -1022,8 +1024,6 @@
$(ECHO) $(THIRD_PARTY_OBJS) | $(TRANSLATE_LINKER_FILE_LIST) >>
$(OUTDIR)/obj_list.temp
$(MKDLL) $(DLLFLAGS) $($(BROWSER)_DLLFLAGS) $($(BROWSER)_LINK_EXTRAS)
$($(BROWSER)_LIBS) $(SKIA_LIB) $(EXT_LINKER_CMD_FLAG)$(OUTDIR)/obj_list.temp
rm $(OUTDIR)/obj_list.temp
-endif # !symbian
-endif # !android
# Note the use of DLLFLAGS_NOPDB instead of DLLFLAGS here.
$(OPERA_WINCESETUP_DLL): $(OPERA_WINCESETUP_OBJS)
$(OPERA_WINCESETUP_LINK_EXTRAS)
==== //depot/googleclient/gears/opensource/gears/ui/ie/ui_resources.rc.m4#8 -
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/ui/ie/ui_resources.rc.m4
====
# action=edit type=text
--- googleclient/gears/opensource/gears/ui/ie/ui_resources.rc.m4
2009-01-16 11:46:14.000000000 +0000
+++ googleclient/gears/opensource/gears/ui/ie/ui_resources.rc.m4
2009-01-15 16:06:23.000000000 +0000
@@ -41,8 +41,9 @@
// using something else? It seems to work as is.
//-----------------------------------------------------------------------------
-button.css HTML "ui/common/button.css"
-button.css.end HTML {"\0END\0"}
+#ifdef WINCE
+// We don't use custom buttons on WinCE.
+#else
button_bg.gif HTML "ui/common/button_bg.gif"
button_bg.gif.end HTML {"\0END\0"}
button_corner_black.gif HTML "ui/common/button_corner_black.gif"
@@ -51,6 +52,8 @@
button_corner_blue.gif.end HTML {"\0END\0"}
button_corner_grey.gif HTML "ui/common/button_corner_grey.gif"
button_corner_grey.gif.end HTML {"\0END\0"}
+#endif
+
html_dialog.css HTML "ui/common/html_dialog.css"
html_dialog.css.end HTML {"\0END\0"}
icon_32x32.png HTML "ui/common/icon_32x32.png"
@@ -60,8 +63,12 @@
location_data.png HTML "ui/common/location_data.png"
location_data.png.end HTML {"\0END\0"}
+#ifdef WINCE
+// We don't use the alert dialog on WinCE.
+#else
alert_dialog.html HTML "genfiles/alert_dialog.html"
alert_dialog.html.end HTML {"\0END\0"}
+#endif
permissions_dialog.html HTML "genfiles/permissions_dialog.html"
permissions_dialog.html.end HTML {"\0END\0"}
settings_dialog.html HTML "genfiles/settings_dialog.html"